[ SEMANAGE ] Stub out user/port functionality

[ SEMANAGE ] Stub out user/port functionality

[Ivan Gyurdiev]

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

The attached patch for libsemanage stubs out the functionality for managing
user and port records. This means simply editing the config files...
As discussed with Joshua, loading the users and ports into policy needs
to be accomplished at commit time, after linking in modules.

I'm still not entirely clear how all the pieces will fit together, but
I think it would be good to merge a stubbed-out skeleton...later we could
change it to make use of the planned semanage_handle_t.

The second patch fixes sepol headers to include stddef.h whenever size_t 
is used.


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

diff -Naur libsemanage/include/semanage/ports.h libsemanage.new/include/semanage/ports.h
--- libsemanage/include/semanage/ports.h	1969-12-31 19:00:00.000000000 -0500
+++ libsemanage.new/include/semanage/ports.h	2005-09-12 07:58:00.000000000 -0400
@@ -0,0 +1,40 @@
+#ifndef _SEMANAGE_PORTS_H_
+#define _SEMANAGE_PORTS_H_
+
+#include <stddef.h>
+#include <semanage/port_record.h>
+
+extern int semanage_port_add(
+	semanage_port_key_t key,
+	semanage_port_t data);
+
+extern int semanage_port_modify(
+	semanage_port_key_t key,
+	semanage_port_t data);
+
+extern int semanage_port_del(
+	semanage_port_key_t key);
+
+extern int semanage_port_query(
+	semanage_port_key_t key,
+	semanage_port_t* response);
+
+extern int semanage_port_exists(
+	semanage_port_key_t key,
+	int* response);
+
+extern int semanage_port_count(
+	int* response);
+
+extern int semanage_port_iterate(
+	int (*handler) (semanage_port_key_t key,
+	                semanage_port_t record,
+	                void* varg),
+	void* handler_arg);
+
+extern int semanage_port_list(
+	semanage_port_key_t** keyset,
+	semanage_port_t** dataset,
+	size_t* count);
+
+#endif 
diff -Naur libsemanage/include/semanage/users.h libsemanage.new/include/semanage/users.h
--- libsemanage/include/semanage/users.h	1969-12-31 19:00:00.000000000 -0500
+++ libsemanage.new/include/semanage/users.h	2005-09-12 07:57:40.000000000 -0400
@@ -0,0 +1,40 @@
+#ifndef _SEMANAGE_USERS_H_
+#define _SEMANAGE_USERS_H_
+
+#include <stddef.h>
+#include <semanage/user_record.h>
+
+extern int semanage_user_add(
+	semanage_user_key_t key,
+	semanage_user_t data);
+
+extern int semanage_user_modify(
+	semanage_user_key_t key,
+	semanage_user_t data);
+
+extern int semanage_user_del(
+	semanage_user_key_t key);
+
+extern int semanage_user_query(
+	semanage_user_key_t key,
+	semanage_user_t* response);
+
+extern int semanage_user_exists(
+	semanage_user_key_t key,
+	int* response);
+
+extern int semanage_user_count(
+	int* response);
+
+extern int semanage_user_iterate(
+	int (*handler) (semanage_user_key_t key,
+	                semanage_user_t record,
+	                void* varg),
+	void* handler_arg);
+
+extern int semanage_user_list(
+	semanage_user_key_t** keyset,
+	semanage_user_t** dataset,
+	size_t* count);
+
+#endif 
diff -Naur libsemanage/src/database_file.c libsemanage.new/src/database_file.c
--- libsemanage/src/database_file.c	1969-12-31 19:00:00.000000000 -0500
+++ libsemanage.new/src/database_file.c	2005-09-12 07:59:48.000000000 -0400
@@ -0,0 +1,106 @@
+#include <stdlib.h>
+#include <stddef.h>
+#include "database.h"
+
+struct dbase_config {
+	/* Stub */
+};
+
+dbase_config_t* dbase[DBASE_COUNT];
+
+int dbase_add(
+	dbase_config_t* dconfig,
+	record_key_t key,
+	record_t data) {
+
+	/* Stub */
+	dconfig = NULL;
+	key = NULL;
+	data = NULL;
+	return -1;
+}
+
+int dbase_modify(
+	dbase_config_t* dconfig,
+	record_key_t key,
+	record_t data) {
+
+	/* Stub */
+	dconfig = NULL;
+	key = NULL;
+	data = NULL;
+	return -1;
+}
+
+int dbase_del(
+	dbase_config_t* dconfig,
+	record_key_t key) {
+	
+	/* Stub */
+	dconfig = NULL;
+	key = NULL;
+	return -1;
+}
+
+int dbase_query(
+	dbase_config_t* dconfig,
+	record_key_t key,
+	record_t* response) {
+	
+	/* Stub */
+	dconfig = NULL;
+	key = NULL;
+	response = NULL;
+	return -1;
+}
+
+int dbase_exists(
+	dbase_config_t* dconfig,
+	record_key_t key,
+	int* response) {
+	
+	/* Stub */
+	dconfig = NULL;
+	key = NULL;
+	response = NULL;
+	return -1;
+}
+
+int dbase_count(
+	dbase_config_t* dconfig,
+	int* response) {
+	
+	/* Stub */
+	dconfig = NULL;
+	response = NULL;
+	return -1;
+}
+
+int dbase_iterate(
+	dbase_config_t* dconfig,
+	int (*handler) (record_key_t key,
+	                record_t record,
+	                void* varg),
+	void* handler_arg) {
+
+	/* Stub */
+	dconfig = NULL;
+	handler = NULL;
+	handler_arg = NULL;
+	return -1;
+}
+
+int dbase_list(
+	dbase_config_t* dconfig,
+	record_key_t** keyset,
+	record_t** dataset,	
+	size_t* count) {
+
+	
+	/* Stub */
+	dconfig = NULL;
+	keyset = NULL;
+	dataset = NULL;
+	count = NULL;
+	return -1;
+}
diff -Naur libsemanage/src/database.h libsemanage.new/src/database.h
--- libsemanage/src/database.h	1969-12-31 19:00:00.000000000 -0500
+++ libsemanage.new/src/database.h	2005-09-12 07:58:35.000000000 -0400
@@ -0,0 +1,61 @@
+#ifndef _SEMANAGE_DATABASE_H_
+#define _SEMANAGE_DATABASE_H_
+
+#include <stddef.h>
+
+#ifndef RECORD_DEFINED
+typedef void* record_t;
+typedef void* record_key_t;
+#define RECORD_DEFINED
+#endif
+
+struct dbase_config;
+typedef struct dbase_config dbase_config_t;
+
+#define DBASE_COUNT 2
+#define DBASE_USERS 0
+#define DBASE_PORTS 1
+extern dbase_config_t* dbase[DBASE_COUNT];
+
+extern int dbase_add(
+	dbase_config_t* dconfig,
+	record_key_t key,
+	record_t data);
+
+extern int dbase_modify(
+	dbase_config_t* dconfig,
+	record_key_t key,
+	record_t data);
+
+extern int dbase_del(
+	dbase_config_t* dconfig,
+	record_key_t key);
+
+extern int dbase_query(
+	dbase_config_t* dconfig,
+	record_key_t key,
+	record_t* response);
+
+extern int dbase_exists(
+	dbase_config_t* dconfig,
+	record_key_t key,
+	int* response);
+
+extern int dbase_count(
+	dbase_config_t* dconfig,
+	int* response);
+
+extern int dbase_iterate(
+	dbase_config_t* dconfig,
+	int (*handler) (record_key_t key,
+	                record_t record,
+	                void* varg),
+	void* handler_arg);
+
+extern int dbase_list(
+	dbase_config_t* dconfig,
+	record_key_t** keyset,
+	record_t** dataset,	
+	size_t* count);
+
+#endif 
diff -Naur libsemanage/src/ports.c libsemanage.new/src/ports.c
--- libsemanage/src/ports.c	1969-12-31 19:00:00.000000000 -0500
+++ libsemanage.new/src/ports.c	2005-09-12 07:59:27.000000000 -0400
@@ -0,0 +1,65 @@
+#include <stddef.h>
+#include <semanage/port_record.h>
+#include <semanage/ports.h>
+
+typedef semanage_port_key_t record_key_t;
+typedef semanage_port_t record_t;
+#define RECORD_DEFINED
+#include "database.h"
+
+int semanage_port_add(
+	semanage_port_key_t key,
+	semanage_port_t data) {
+	
+	return dbase_add(dbase[DBASE_PORTS], key, data);
+}
+
+int semanage_port_modify(
+	semanage_port_key_t key,
+	semanage_port_t data) {
+	
+	return dbase_modify(dbase[DBASE_PORTS], key, data);
+}
+
+int semanage_port_del(
+	semanage_port_key_t key) {
+
+	return dbase_del(dbase[DBASE_PORTS], key);
+}
+
+int semanage_port_query(
+	semanage_port_key_t key,
+	semanage_port_t* response) {
+
+	return dbase_query(dbase[DBASE_PORTS], key, response);
+}
+
+int semanage_port_exists(
+	semanage_port_key_t key,
+	int* response) {
+
+	return dbase_exists(dbase[DBASE_PORTS], key, response);
+}
+
+int semanage_port_count(
+	int* response) {
+
+	return dbase_count(dbase[DBASE_PORTS], response);
+}
+
+int semanage_port_iterate(
+	int (*handler) (semanage_port_key_t key,
+	                semanage_port_t record,
+	                void* varg),
+	void* handler_arg) {
+
+	return dbase_iterate(dbase[DBASE_PORTS], handler, handler_arg);
+}
+
+int semanage_port_list(
+	semanage_port_key_t** keyset,
+	semanage_port_t** dataset,
+	size_t* count) {
+
+	return dbase_list(dbase[DBASE_PORTS], keyset, dataset, count);
+}
diff -Naur libsemanage/src/users.c libsemanage.new/src/users.c
--- libsemanage/src/users.c	1969-12-31 19:00:00.000000000 -0500
+++ libsemanage.new/src/users.c	2005-09-12 07:59:04.000000000 -0400
@@ -0,0 +1,65 @@
+#include <stddef.h>
+#include <semanage/user_record.h>
+#include <semanage/users.h>
+
+typedef semanage_user_key_t record_key_t;
+typedef semanage_user_t record_t;
+#define RECORD_DEFINED
+#include "database.h"
+
+int semanage_user_add(
+	semanage_user_key_t key,
+	semanage_user_t data) {
+	
+	return dbase_add(dbase[DBASE_USERS], key, data);
+}
+
+int semanage_user_modify(
+	semanage_user_key_t key,
+	semanage_user_t data) {
+	
+	return dbase_modify(dbase[DBASE_USERS], key, data);
+}
+
+int semanage_user_del(
+	semanage_user_key_t key) {
+
+	return dbase_del(dbase[DBASE_USERS], key);
+}
+
+int semanage_user_query(
+	semanage_user_key_t key,
+	semanage_user_t* response) {
+
+	return dbase_query(dbase[DBASE_USERS], key, response);
+}
+
+int semanage_user_exists(
+	semanage_user_key_t key,
+	int* response) {
+
+	return dbase_exists(dbase[DBASE_USERS], key, response);
+}
+
+int semanage_user_count(
+	int* response) {
+
+	return dbase_count(dbase[DBASE_USERS], response);
+}
+
+int semanage_user_iterate(
+	int (*handler) (semanage_user_key_t key,
+	                semanage_user_t record,
+	                void* varg),
+	void* handler_arg) {
+
+	return dbase_iterate(dbase[DBASE_USERS], handler, handler_arg);
+}
+
+int semanage_user_list(
+	semanage_user_key_t** keyset,
+	semanage_user_t** dataset,
+	size_t* count) {
+
+	return dbase_list(dbase[DBASE_USERS], keyset, dataset, count);
+}

[-- Attachment #3: libsepol.stddef.diff --]
[-- Type: text/x-patch, Size: 4507 bytes --]

diff -Naur libsepol/include/sepol/context.h libsepol.new/include/sepol/context.h
--- libsepol/include/sepol/context.h	2005-08-21 12:56:15.000000000 -0400
+++ libsepol.new/include/sepol/context.h	2005-09-12 07:40:39.000000000 -0400
@@ -19,6 +19,7 @@
 #ifndef _SEPOL_CONTEXT_H_
 #define _SEPOL_CONTEXT_H_
 
+#include <stddef.h>
 #include <sepol/ebitmap.h>
 #include <sepol/mls_types.h>
 #include <sepol/context_record.h>
diff -Naur libsepol/include/sepol/expand.h libsepol.new/include/sepol/expand.h
--- libsepol/include/sepol/expand.h	2005-08-21 12:56:16.000000000 -0400
+++ libsepol.new/include/sepol/expand.h	2005-09-12 07:40:34.000000000 -0400
@@ -22,6 +22,7 @@
  *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
+#include <stddef.h>
 #include <sepol/conditional.h>
 
 #ifndef _SEPOL_EXPAND_H
diff -Naur libsepol/include/sepol/interfaces.h libsepol.new/include/sepol/interfaces.h
--- libsepol/include/sepol/interfaces.h	2005-08-21 12:56:16.000000000 -0400
+++ libsepol.new/include/sepol/interfaces.h	2005-09-12 07:40:13.000000000 -0400
@@ -1,5 +1,6 @@
 #include <sepol/policydb.h>
 #include <sepol/context_record.h>
+#include <stddef.h>
 
 /* High level representation of an interface */
 typedef struct sepol_iface {
diff -Naur libsepol/include/sepol/link.h libsepol.new/include/sepol/link.h
--- libsepol/include/sepol/link.h	2005-07-13 15:42:37.000000000 -0400
+++ libsepol.new/include/sepol/link.h	2005-09-12 07:40:23.000000000 -0400
@@ -23,6 +23,7 @@
  */
 
 #include <sepol/policydb.h>
+#include <stddef.h>
 
 #ifndef _SEPOL_LINK_H
 #define _SEPOL_LINK_H
diff -Naur libsepol/include/sepol/module.h libsepol.new/include/sepol/module.h
--- libsepol/include/sepol/module.h	2005-08-02 15:41:19.000000000 -0400
+++ libsepol.new/include/sepol/module.h	2005-09-12 07:40:19.000000000 -0400
@@ -21,6 +21,7 @@
 #define _SEPOL_MODULE_H_
 
 #include <stdlib.h>
+#include <stddef.h>
 
 #include <sepol/policydb.h>
 #include <sepol/conditional.h>
diff -Naur libsepol/include/sepol/policydb.h libsepol.new/include/sepol/policydb.h
--- libsepol/include/sepol/policydb.h	2005-08-21 12:56:16.000000000 -0400
+++ libsepol.new/include/sepol/policydb.h	2005-09-12 07:39:43.000000000 -0400
@@ -52,6 +52,7 @@
 #define _POLICYDB_H_
 
 #include <stdio.h>
+#include <stddef.h>
 
 #include <sepol/flask_types.h>
 #include <sepol/symtab.h>
diff -Naur libsepol/include/sepol/ports.h libsepol.new/include/sepol/ports.h
--- libsepol/include/sepol/ports.h	2005-08-21 12:56:16.000000000 -0400
+++ libsepol.new/include/sepol/ports.h	2005-09-12 07:39:29.000000000 -0400
@@ -3,6 +3,7 @@
 
 #include <sepol/policydb.h>
 #include <sepol/port_record.h>
+#include <stddef.h>
 
 /* Create a port structure from high level representation */
 extern int sepol_port_struct_create(
diff -Naur libsepol/include/sepol/sepol.h libsepol.new/include/sepol/sepol.h
--- libsepol/include/sepol/sepol.h	2005-04-13 10:56:10.000000000 -0400
+++ libsepol.new/include/sepol/sepol.h	2005-09-12 07:39:15.000000000 -0400
@@ -1,7 +1,7 @@
 #ifndef _SEPOL_H_
 #define _SEPOL_H_
 
-#include <sys/types.h>
+#include <stddef.h>
 #include <stdio.h>
 
 /* Given an existing binary policy (starting at 'data', with length 'len')
diff -Naur libsepol/include/sepol/services.h libsepol.new/include/sepol/services.h
--- libsepol/include/sepol/services.h	2005-07-13 15:42:37.000000000 -0400
+++ libsepol.new/include/sepol/services.h	2005-09-12 07:37:54.000000000 -0400
@@ -14,6 +14,7 @@
 
 #include <sepol/flask_types.h>
 #include <sepol/policydb.h>
+#include <stddef.h>
 
 /* Set the policydb and sidtab structures to be used by
    the service functions.  If not set, then these default
diff -Naur libsepol/include/sepol/user_record.h libsepol.new/include/sepol/user_record.h
--- libsepol/include/sepol/user_record.h	2005-08-02 09:16:53.000000000 -0400
+++ libsepol.new/include/sepol/user_record.h	2005-09-12 07:36:40.000000000 -0400
@@ -1,6 +1,8 @@
 #ifndef _SEPOL_USER_RECORD_H_
 #define _SEPOL_USER_RECORD_H_
 
+#include <stddef.h>
+
 struct sepol_user;
 struct sepol_user_key;
 typedef struct sepol_user* sepol_user_t;
diff -Naur libsepol/include/sepol/users.h libsepol.new/include/sepol/users.h
--- libsepol/include/sepol/users.h	2005-08-21 12:56:16.000000000 -0400
+++ libsepol.new/include/sepol/users.h	2005-09-12 07:36:45.000000000 -0400
@@ -3,7 +3,7 @@
 
 #include <sepol/policydb.h>
 #include <sepol/user_record.h>
-#include <sys/types.h>
+#include <stddef.h>
 
 /* Clear unused users */
 extern void sepol_clear_unused_users(

Re: [ SEMANAGE ] Introduce record table

[Ivan Gyurdiev]

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

The attached patch applies on top of the other one.
It introduces the record table as the first thing
which will be stored in the database config (a database config
stores parameters specific to a database (a.k.a. file))

To clarify how this works - there's two sets of files here - one
that works with generic record_t/record_key_t objects (treated as void*),
and one that works with specific types that replace record_t/record_key_t.
This essentially implements inheritance, which allows sharing of a lot of
code (not yet merged). It's also a bit tricky to get it working, because we
must keep track of where the specific types are defined.

The record table stores a set of functions which correspond
to the record headers previously merged. It defines how certain
operations are done for each particular type of record. In addition, it 
provides
some functions specific to the FILE backend. This table will
be used by the file engine for polymorphism - it will work with
the generic record types, but will be invoking the specific functions for
each record based on what the table says.

Regarding database_init/close - those need to be integrated somewhere.. 
likely
on CONNECT/DISCONNECT...but I haven't done that yet..and it might change
once semanage_handle_t appears.

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

diff -Naur libsemanage/src/database_file.c libsemanage.new/src/database_file.c
--- libsemanage/src/database_file.c	2005-09-12 09:56:33.000000000 -0400
+++ libsemanage.new/src/database_file.c	2005-09-12 09:49:33.000000000 -0400
@@ -1,13 +1,41 @@
 #include <stdlib.h>
 #include <stddef.h>
 #include "database.h"
+#include "record_file.h"
+#include "users_file.h"
+#include "ports_file.h"
 
 struct dbase_config {
-	/* Stub */
+	record_table_t* rtable;
 };
 
 dbase_config_t* dbase[DBASE_COUNT];
 
+int dbase_init() {
+	int i;
+	for (i = 0; i < DBASE_COUNT; i++) {
+		dbase[i] = (dbase_config_t*) malloc(sizeof(dbase_config_t));
+		if (dbase[i] == NULL) 
+			goto err;
+	}
+
+	dbase[DBASE_USERS]->rtable = &RTABLE_USER;
+	dbase[DBASE_PORTS]->rtable = &RTABLE_PORT; 	
+
+	return 0;
+	
+	err:
+	for (i--; i >= 0; i--) 
+		free(dbase[i]);
+	return -1;
+}	
+
+void dbase_close() {
+	int i;
+	for (i = 0; i < DBASE_COUNT; i++)
+		free(dbase[i]);
+}
+
 int dbase_add(
 	dbase_config_t* dconfig,
 	record_key_t key,
diff -Naur libsemanage/src/ports_file.c libsemanage.new/src/ports_file.c
--- libsemanage/src/ports_file.c	1969-12-31 19:00:00.000000000 -0500
+++ libsemanage.new/src/ports_file.c	2005-09-12 09:51:39.000000000 -0400
@@ -0,0 +1,42 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <semanage/port_record.h>
+
+typedef semanage_port_t record_t;
+typedef semanage_port_key_t record_key_t;
+#define RECORD_DEFINED
+#include "record_file.h"
+
+static int semanage_port_print(
+	semanage_port_t port, 
+	FILE* str) {
+
+	/* Stub */
+	port = NULL;
+	str = NULL;
+	return -1;
+}
+
+static int semanage_port_parse(
+	parse_info_t* info, 
+	semanage_port_t port) {
+
+	/* Stub */
+	info = NULL;
+	port = NULL;
+	return -1;	
+}
+
+record_table_t RTABLE_PORT = {
+	/* Record base functions */
+	.create      = semanage_port_create,
+	.key_extract = semanage_port_key_extract,
+	.key_free    = semanage_port_key_free,
+	.clone       = semanage_port_clone,
+	.compare     = semanage_port_compare,
+	.free        = semanage_port_free,
+
+	/* Record functions for FILE backend */
+	.parse       = semanage_port_parse,
+	.print       = semanage_port_print,
+};
diff -Naur libsemanage/src/ports_file.h libsemanage.new/src/ports_file.h
--- libsemanage/src/ports_file.h	1969-12-31 19:00:00.000000000 -0500
+++ libsemanage.new/src/ports_file.h	2005-09-12 09:51:10.000000000 -0400
@@ -0,0 +1,8 @@
+#ifndef _SEMANAGE_PORTS_FILE_H_
+#define _SEMANAGE_PORTS_FILE_H_
+
+#include "record_file.h"
+
+extern record_table_t RTABLE_PORT;
+
+#endif 
diff -Naur libsemanage/src/record_file.h libsemanage.new/src/record_file.h
--- libsemanage/src/record_file.h	1969-12-31 19:00:00.000000000 -0500
+++ libsemanage.new/src/record_file.h	2005-09-12 09:38:06.000000000 -0400
@@ -0,0 +1,47 @@
+#ifndef _SEMANAGE_RECORD_FILE_H_
+#define _SEMANAGE_RECORD_FILE_H_
+
+#include <stdio.h>
+
+#ifndef RECORD_DEFINED
+typedef void* record_t;
+typedef void* record_key_t;
+#define RECORD_DEFINED
+#endif
+
+/* Structure available during parsing (created internally) */
+typedef struct parse_info {
+	/* Stub */	
+} parse_info_t;
+
+/* Record table format - necessary during processing */
+typedef struct record_table {
+
+	/* Create a record */
+	int (*create) (record_t* rec);
+
+	/* Extract key from record */
+	int (*key_extract) (record_t rec, record_key_t* key);
+	
+	/* Free record key */
+	int (*key_free) (record_key_t key);
+
+	/* Return 0 if record can be matched against key,
+	 * and 1 otherwise */
+	int (*compare) (record_t rec, record_key_t key);
+
+	/* Deep-copy clone of this record */
+	int (*clone) (record_t rec, record_t* new_rec);
+
+	/* Fill record structuure based on supplied parse info */
+	int (*parse) (parse_info_t* info, record_t record);
+
+	/* Print record to stream */
+	int (*print) (record_t record, FILE* str);
+
+	/* Deallocate record resources. Must
+	 * sucessfully handle NULL. */
+	void (*free) (record_t rec);
+} record_table_t;
+
+#endif 
diff -Naur libsemanage/src/users_file.c libsemanage.new/src/users_file.c
--- libsemanage/src/users_file.c	1969-12-31 19:00:00.000000000 -0500
+++ libsemanage.new/src/users_file.c	2005-09-12 09:51:52.000000000 -0400
@@ -0,0 +1,42 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <semanage/user_record.h>
+
+typedef semanage_user_t record_t;
+typedef semanage_user_key_t record_key_t;
+#define RECORD_DEFINED
+#include "record_file.h"
+
+static int semanage_user_print(
+	semanage_user_t user, 
+	FILE* str) {
+
+	/* Stub */
+	user = NULL;
+	str = NULL;
+	return -1;
+}
+
+static int semanage_user_parse(
+	parse_info_t* info, 
+	semanage_user_t user) {
+
+	/* Stub */
+	info = NULL;
+	user = NULL;
+	return -1;	
+}
+
+record_table_t RTABLE_USER = {
+	/* Record base functions */
+	.create      = semanage_user_create,
+	.key_extract = semanage_user_key_extract,
+	.key_free    = semanage_user_key_free,
+	.clone       = semanage_user_clone,
+	.compare     = semanage_user_compare,
+	.free        = semanage_user_free,
+
+	/* Record functions for FILE backend */
+	.parse       = semanage_user_parse,
+	.print       = semanage_user_print,
+};
diff -Naur libsemanage/src/users_file.h libsemanage.new/src/users_file.h
--- libsemanage/src/users_file.h	1969-12-31 19:00:00.000000000 -0500
+++ libsemanage.new/src/users_file.h	2005-09-12 09:49:53.000000000 -0400
@@ -0,0 +1,8 @@
+#ifndef _SEMANAGE_USERS_FILE_H_
+#define _SEMANAGE_USERS_FILE_H_
+
+#include "record_file.h"
+
+extern record_table_t RTABLE_USER;
+
+#endif 

Re: [ SEPOL ] Move more things to newer debug system

[Ivan Gyurdiev]

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

The attached patch changes printf/fprintf usage in policydb.c, and
avtab.c to the new debug system. This allows the user to set the print 
function.
It also writes to stderr by default, as opposed to stdout (which is 
wrong, imho).
As a side effect, this will append a more verbose prefix everywhere
that gives the exact function name, rather than "security : mls" or 
whatever.

This also removes the __sepol_debug_printf debug system from
gen*, and changes genbools and genusers to use the newer system.

I haven't really tested this, but it's fairly straightforward, and I don't
see what can break - it compiles without warnings.

============
We need to reduce the number of debug systems in the selinux libraries.
printf's should be eradicated. There's also the alternative system that 
semanage
uses, based on buffering the errors - I'm not clear how that will
be made to work in concert with sepol debugging.



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

diff -Naur libsepol/src/avtab.c libsepol.new/src/avtab.c
--- libsepol/src/avtab.c	2005-08-21 12:56:16.000000000 -0400
+++ libsepol.new/src/avtab.c	2005-09-12 23:30:54.000000000 -0400
@@ -37,6 +37,7 @@
 #include <sepol/avtab.h>
 #include <sepol/policydb.h>
 
+#include "debug.h"
 #include "private.h"
 
 #define AVTAB_HASH(keyp) \
@@ -357,19 +358,19 @@
 	if (vers < POLICYDB_VERSION_AVTAB) {
 		buf32 = next_entry(fp, sizeof(uint32_t));
 		if (!buf32) {
-			printf("security: avtab: truncated entry\n");
+			DEBUG(__FUNCTION__, "truncated entry\n");
 			return -1;
 		}
 		items2 = le32_to_cpu(buf32[0]);
 
 		if (items2 < 5 || items2 > 8) {
-			printf("security: avtab: invalid item count\n");
+			DEBUG(__FUNCTION__, "invalid item count\n");
 			return -1;
 		}
 
 		buf32 = next_entry(fp, sizeof(uint32_t)*items2);
 		if (!buf32) {
-			printf("security: avtab: truncated entry\n");
+			DEBUG(__FUNCTION__, "truncated entry\n");
 			return -1;
 		}
 
@@ -377,19 +378,19 @@
 		val = le32_to_cpu(buf32[items++]);
 		key.source_type = (uint16_t)val;
 		if (key.source_type != val) {
-			printf("security: avtab: truncated source type\n");
+			DEBUG(__FUNCTION__, "truncated source type\n");
 			return -1;
 		}
 		val = le32_to_cpu(buf32[items++]);
 		key.target_type = (uint16_t)val;
 		if (key.target_type != val) {
-			printf("security: avtab: truncated target type\n");
+			DEBUG(__FUNCTION__, "truncated target type\n");
 			return -1;
 		}
 		val = le32_to_cpu(buf32[items++]);
 		key.target_class = (uint16_t)val;
 		if (key.target_class != val) {
-			printf("security: avtab: truncated target class\n");
+			DEBUG(__FUNCTION__, "truncated target class\n");
 			return -1;
 		}
 
@@ -397,12 +398,13 @@
 		enabled = (val & AVTAB_ENABLED_OLD) ? AVTAB_ENABLED : 0;
 
 		if (!(val & (AVTAB_AV | AVTAB_TYPE))) {
-			printf("security: avtab: null entry\n");
+			DEBUG(__FUNCTION__, "null entry\n");
 			return -1;
 		}
 		if ((val & AVTAB_AV) &&
 		    (val & AVTAB_TYPE)) {
-			printf("security: avtab: entry has both access vectors and types\n");
+			DEBUG(__FUNCTION__, "entry has both access "
+				"vectors and types\n");
 			return -1;
 		}
 
@@ -416,7 +418,8 @@
 		}
 
 		if (items != items2) {
-			printf("security: avtab: entry only had %d items, expected %d\n", items2, items);
+			DEBUG(__FUNCTION__, "entry only had %d items, "
+				"expected %d\n", items2, items);
 			return -1;
 		}
 		return 0;
@@ -424,7 +427,7 @@
 	
 	buf16 = next_entry(fp, sizeof(uint16_t)*4);
 	if (!buf16) {
-		printf("security: avtab: truncated entry\n");
+		DEBUG(__FUNCTION__, "truncated entry\n");
 		return -1;
 	}
 	items = 0;
@@ -439,13 +442,13 @@
 				set++;
 	}
 	if (!set || set > 1) {
-		printf("security: avtab: more than one specifier\n");
+		DEBUG(__FUNCTION__, "more than one specifier\n");
 		return -1;
 	}
 		
 	buf32 = next_entry(fp, sizeof(uint32_t));
 	if (!buf32) {
-		printf("security: avtab: truncated entry\n");
+		DEBUG(__FUNCTION__, "truncated entry\n");
 		return -1;
 	}
 	datum.data = le32_to_cpu(*buf32);
@@ -467,22 +470,22 @@
 
 	buf = next_entry(fp, sizeof(uint32_t));
 	if (!buf) {
-		printf("security: avtab: truncated table\n");
+		DEBUG(__FUNCTION__, "truncated table\n");
 		goto bad;
 	}
 	nel = le32_to_cpu(buf[0]);
 	if (!nel) {
-		printf("security: avtab: table is empty\n");
+		DEBUG(__FUNCTION__, "table is empty\n");
 		goto bad;
 	}
 	for (i = 0; i < nel; i++) {
 		rc = avtab_read_item(fp, vers, a, avtab_insertf, NULL);
 		if (rc) {
 			if (rc == -ENOMEM)
-				printf("security: avtab: out of memory\n");
+				DEBUG(__FUNCTION__, "out of memory\n");
 			if (rc == -EEXIST)
-				printf("security: avtab: duplicate entry\n");
-			printf("Failed on entry %d of %u\n", i, nel);
+				DEBUG(__FUNCTION__, "duplicate entry\n");
+			DEBUG(__FUNCTION__, "failed on entry %d of %u\n", i, nel);
 			goto bad;
 		}
 	}
diff -Naur libsepol/src/debug.c libsepol.new/src/debug.c
--- libsepol/src/debug.c	2005-07-18 10:28:43.000000000 -0400
+++ libsepol.new/src/debug.c	2005-09-12 23:41:51.000000000 -0400
@@ -31,6 +31,11 @@
 
 void (*DEBUG) (const char* fname, const char* fmt, ...) = default_printf;
 
+/* Compatibility */
+void sepol_debug(int on) {
+        sepol_debug_compat(on);
+};
+
 void sepol_debug_compat(int on) {
 	DEBUG = (on)? default_printf : suppress_printf;
 }
diff -Naur libsepol/src/genbools.c libsepol.new/src/genbools.c
--- libsepol/src/genbools.c	2005-08-31 16:51:16.000000000 -0400
+++ libsepol.new/src/genbools.c	2005-09-12 23:40:43.000000000 -0400
@@ -6,6 +6,7 @@
 #include <sepol/policydb.h>
 #include <sepol/conditional.h>
 
+#include "debug.h"
 #include "private.h"
 
 static char *strtrim(char *dest, char *source, int size) {
@@ -43,7 +44,8 @@
 			else if (!strncasecmp(tok, "false", sizeof("false")-1))
 				*val = 0;
 			if (*val != 0 && *val != 1) {
-				fprintf(stderr,"illegal value for boolean %s=%s\n", name, tok);
+				DEBUG(__FUNCTION__, "illegal value for boolean "
+					"%s=%s\n", name, tok);
 				return -1;
 			}
 			
@@ -73,7 +75,7 @@
 		if (ret==1) {
 			datum = hashtab_search(policydb->p_bools.table, name);
 			if (!datum) {
-				fprintf(stderr,"unknown boolean %s\n", name);
+				DEBUG(__FUNCTION__, "unknown boolean %s\n", name);
 				errors++;
 				continue;
 			}
@@ -92,7 +94,7 @@
 			if (ret==1) {
 				datum = hashtab_search(policydb->p_bools.table, name);
 				if (!datum) {
-					fprintf(stderr,"unknown boolean %s\n", name);
+					DEBUG(__FUNCTION__, "unknown boolean %s\n", name);
 					errors++;
 					continue;
 				}
@@ -122,13 +124,12 @@
 	sepol_set_policyvers(policydb.policy_type, policydb.policyvers);
 
 	if (load_booleans(&policydb, booleans) < 0) {
-		__sepol_debug_printf("%s:  Warning!  Error while reading %s\n",
-				     __FUNCTION__, booleans);
+		DEBUG(__FUNCTION__, "Warning! Error while reading %s\n",
+				     booleans);
 	}
 
 	if (evaluate_conds(&policydb) < 0) {
-		__sepol_debug_printf("%s:  Error while re-evaluating conditionals\n",
-				     __FUNCTION__);
+		DEBUG(__FUNCTION__, "error while re-evaluating conditionals\n");
 		errno = EINVAL;
 		goto err_destroy;
 	}
@@ -138,8 +139,7 @@
 	pf.len = len;
 	rc = policydb_write(&policydb, &pf);
 	if (rc) {
-		__sepol_debug_printf("%s: Can't write new binary policy image\n",
-				     __FUNCTION__);
+		DEBUG(__FUNCTION__, "unable to write new binary policy image\n");
 		errno = EINVAL;
 		goto err_destroy;
 	}
@@ -184,13 +184,14 @@
 	for (i = 0; i < nel; i++) {
 		datum = hashtab_search(policydb.p_bools.table, names[i]);
 		if (!datum) {
-			__sepol_debug_printf("%s:  boolean %s no longer in policy\n", 
-					     __FUNCTION__, names[i]);
+			DEBUG(__FUNCTION__, "boolean %s no longer in policy\n",
+				names[i]);
 			errors++;
 			continue;
 		}
 		if (values[i] != 0 && values[i] != 1) {
-			fprintf(stderr,"illegal value %d for boolean %s\n", values[i], names[i]);
+			DEBUG(__FUNCTION__, "illegal value %d for boolean %s\n",
+				values[i], names[i]);
 			errors++;
 			continue;
 		}
@@ -198,8 +199,7 @@
 	}
 
 	if (evaluate_conds(&policydb) < 0) {
-		__sepol_debug_printf("%s:  Error while re-evaluating conditionals\n",
-				     __FUNCTION__);
+		DEBUG(__FUNCTION__, "error while re-evaluating conditionals\n");
 		errno = EINVAL;
 		goto err_destroy;
 	}
@@ -209,8 +209,7 @@
 	pf.len = len;
 	rc = policydb_write(&policydb, &pf);
 	if (rc) {
-		__sepol_debug_printf("%s:  Can't write binary policy\n",
-				     __FUNCTION__);
+		DEBUG(__FUNCTION__, "unable to write binary policy\n");
 		errno = EINVAL;
 		goto err_destroy;
 	}
diff -Naur libsepol/src/genusers.c libsepol.new/src/genusers.c
--- libsepol/src/genusers.c	2005-08-31 16:51:16.000000000 -0400
+++ libsepol.new/src/genusers.c	2005-09-12 23:43:51.000000000 -0400
@@ -12,33 +12,12 @@
 #include "debug.h"
 #include "private.h"
 
-static int gdebug=1;
-
-void sepol_debug(int on) { 
-	gdebug=on; 
-
-	/* New debug system */
-	sepol_debug_compat(on);
-};
-
-#ifdef __GNUC__
-__attribute__ ((format (printf, 1, 2)))
-#endif
-void __sepol_debug_printf(const char *fmt, ...) {
-	if (gdebug) {
-		va_list ap;
-		va_start(ap, fmt);
-		vfprintf (stderr, fmt, ap);
-		va_end(ap);
-	}
-}
-
 extern int selinux_delusers;
 
 #undef BADLINE
 #define BADLINE() { \
-	__sepol_debug_printf("%s:  invalid entry %s on line %u\n", \
-		path, buffer, lineno); \
+	DEBUG(__FUNCTION__, "invalid entry %s (%s:%u)\n", \
+		buffer, path, lineno); \
 	continue; \
 }
 
@@ -96,8 +75,7 @@
 			/* Adding a new user definition. */
 			usrdatum = (user_datum_t *) malloc(sizeof(user_datum_t));
 			if (!id || !usrdatum) {
-				__sepol_debug_printf("%s:  out of memory for %s on line %u\n",
-					path, buffer, lineno);
+				DEBUG(__FUNCTION__,"out of memory\n");
 				errno = ENOMEM;
 				free(buffer);
 				fclose(fp);
@@ -110,8 +88,7 @@
 			rc = hashtab_insert(policydb->p_users.table,
 					    id, (hashtab_datum_t) usrdatum);
 			if (rc) {
-				__sepol_debug_printf("%s:  out of memory for %s on line %u\n",
-					path, buffer, lineno);
+				DEBUG(__FUNCTION__, "out of memory\n");
 				errno = ENOMEM;
 				free(buffer);
 				fclose(fp);
@@ -158,16 +135,15 @@
 
 			roldatum = hashtab_search(policydb->p_roles.table, q);
 			if (!roldatum) {
-				__sepol_debug_printf("%s:  undefined role %s in %s on line %u\n",
-					path, q, buffer, lineno);
+				DEBUG(__FUNCTION__, "undefined role %s (%s:%u)\n",
+					q, path, lineno);
 				continue;
 			}
 			/* Set the role and every role it dominates */
 			ebitmap_for_each_bit(&roldatum->dominates, rnode, bit) {
 				if (ebitmap_node_get_bit(rnode, bit))
 					if (ebitmap_set_bit(&usrdatum->roles.roles, bit, 1)) {
-						__sepol_debug_printf("%s:  out of memory for %s on line %u\n",
-							path, buffer, lineno);
+						DEBUG(__FUNCTION__, "out of memory\n");
 						errno = ENOMEM;
 						free(buffer);
 						fclose(fp);
@@ -203,9 +179,7 @@
 
 			scontext = malloc(p - q);
 			if (!scontext) {
-				__sepol_debug_printf("%s:  out of memory for %s on line %u\n",
-					path, buffer, lineno);
-				errno = ENOMEM;
+				DEBUG(__FUNCTION__, "out of memory\n");
 				free(buffer);
 				fclose(fp);
 				return -1;
@@ -223,8 +197,8 @@
 			context_init(&context);
 			rc = mls_context_to_sid(policydb, oldc, &r, &context);
 			if (rc) {
-				__sepol_debug_printf("%s:  invalid level %s in %s on line %u\n",
-					path, scontext, buffer, lineno);
+				DEBUG(__FUNCTION__, "invalid level %s (%s:%u)\n",
+					scontext, path, lineno);
 				free(scontext);
 				continue;
 
@@ -250,8 +224,7 @@
 
 			scontext = malloc(p - q);
 			if (!scontext) {
-				__sepol_debug_printf("%s:  out of memory for %s on line %u\n",
-					path, buffer, lineno);
+				DEBUG(__FUNCTION__, "out of memory\n");
 				errno = ENOMEM;
 				free(buffer);
 				fclose(fp);
@@ -270,8 +243,8 @@
 			context_init(&context);
 			rc = mls_context_to_sid(policydb, oldc, &r, &context);
 			if (rc) {
-				__sepol_debug_printf("%s:  invalid range %s in %s on line %u\n",
-					path, scontext, buffer, lineno);
+				DEBUG(__FUNCTION__, "invalid range %s (%s:%u)\n",
+					scontext, path, lineno);
 				free(scontext);
 				continue;
 			}
@@ -362,16 +335,16 @@
 	/* Load base set of system users from the policy package. */
 	snprintf(path, sizeof path, "%s/system.users", usersdir);
 	if (load_users(&policydb, path) < 0) {
-		__sepol_debug_printf("%s: Can't load system.users:  %s\n",
-				     __FUNCTION__, strerror(errno));
+		DEBUG(__FUNCTION__, "unable to load system.users: %s\n",
+				strerror(errno));
 		goto err_destroy;
 	}
 
 	/* Load locally defined users. */
 	snprintf(path, sizeof path, "%s/local.users", usersdir);
 	if (load_users(&policydb, path) < 0) {
-		__sepol_debug_printf("%s:  Can't load local.users:  %s\n",
-				     __FUNCTION__, strerror(errno));
+		DEBUG(__FUNCTION__, "unable to load local.users: %s\n",
+				strerror(errno));
 		goto err_destroy;
 	}
 
@@ -407,22 +380,22 @@
 	/* Load base set of system users from the policy package. */
 	snprintf(path, sizeof path, "%s/system.users", usersdir);
 	if (load_users(policydb, path) < 0) {
-		__sepol_debug_printf("%s: Can't load system.users:  %s\n",
-				     __FUNCTION__, strerror(errno));
+		DEBUG(__FUNCTION__, "unable to load system.users: %s\n",
+				strerror(errno));
 		return -1;
 	}
 
 	/* Load locally defined users. */
 	snprintf(path, sizeof path, "%s/local.users", usersdir);
 	if (load_users(policydb, path) < 0) {
-		__sepol_debug_printf("%s:  Can't load local.users:  %s\n",
-				     __FUNCTION__, strerror(errno));
+		DEBUG(__FUNCTION__, "unable to load local.users: %s\n",
+				strerror(errno));
 		return -1;
 	}
 
 	if (policydb_reindex_users(policydb) < 0) {
-		__sepol_debug_printf("%s:  Can't reindex users:  %s\n",
-				     __FUNCTION__, strerror(errno));
+		DEBUG(__FUNCTION__, "unable to reindex users: %s\n",
+				strerror(errno));
 		return -1;
 
 	}
diff -Naur libsepol/src/policydb.c libsepol.new/src/policydb.c
--- libsepol/src/policydb.c	2005-08-31 16:51:17.000000000 -0400
+++ libsepol.new/src/policydb.c	2005-09-12 23:27:29.000000000 -0400
@@ -1005,20 +1005,20 @@
 	ocontext_t *head, *c;
 
 	if (sepol_sidtab_init(s)) {
-		printf("security:  out of memory on SID table init\n");
+		DEBUG(__FUNCTION__, "out of memory on SID table init\n");
 		return -1;
 	}
 
 	head = p->ocontexts[OCON_ISID];
 	for (c = head; c; c = c->next) {
 		if (!c->context[0].user) {
-			printf("security:  SID %s was never defined.\n", 
-			       c->u.name);
+			DEBUG(__FUNCTION__, "SID %s was never defined\n",
+				c->u.name);
 			return -1;
 		}
 		if (sepol_sidtab_insert(s, c->sid[0], &c->context[0])) {
-			printf("security:  unable to load initial SID %s.\n", 
-			       c->u.name);
+			DEBUG(__FUNCTION__, "unable to load initial SID %s\n",
+				c->u.name);
 			return -1;
 		}
 	}
@@ -1078,7 +1078,7 @@
 	items = le32_to_cpu(buf[0]);
 	buf = next_entry(fp, sizeof(uint32_t)*items);
 	if (!buf) {
-		printf("security: mls:  truncated range\n");
+		DEBUG(__FUNCTION__, "truncated range\n");
 		goto out;
 	}
 	r->level[0].sens = le32_to_cpu(buf[0]);
@@ -1089,19 +1089,19 @@
 
 	rc = ebitmap_read(&r->level[0].cat, fp);
 	if (rc) {
-		printf("security: mls:  error reading low categories\n");
+		DEBUG(__FUNCTION__, "error reading low categories\n");
 		goto out;
 	}
 	if (items > 1) {
 		rc = ebitmap_read(&r->level[1].cat, fp);
 		if (rc) {
-			printf("security: mls:  error reading high categories\n");
+			DEBUG(__FUNCTION__, "error reading high categories\n");
 			goto bad_high;
 		}
 	} else {
 		rc = ebitmap_cpy(&r->level[1].cat, &r->level[0].cat);
 		if (rc) {
-			printf("security: mls:  out of memory\n");
+			DEBUG(__FUNCTION__, "out of memory\n");
 			goto bad_high;
 		}
 	}
@@ -1127,7 +1127,7 @@
 
 	buf = next_entry(fp, sizeof(uint32_t)*3);
 	if (!buf) {
-		printf("security: context truncated\n");
+		DEBUG(__FUNCTION__, "context truncated\n");
 		return -1;
 	}
 	c->user = le32_to_cpu(buf[0]);
@@ -1135,14 +1135,14 @@
 	c->type = le32_to_cpu(buf[2]);
 	if (p->policyvers >= POLICYDB_VERSION_MLS) {
 		if (mls_read_range_helper(&c->range, fp)) {
-			printf("security: error reading MLS range of "
-			       "context\n");
+			DEBUG(__FUNCTION__, "error reading MLS range "
+				"of context\n");
 			return -1;
 		}
 	}
 
 	if (!policydb_context_isvalid(p, c)) {
-		printf("security:  invalid security context\n");
+		DEBUG(__FUNCTION__, "invalid security context\n");
 		context_destroy(c);
 		return -1;
 	}
@@ -1402,7 +1402,8 @@
 		cladatum->comdatum = hashtab_search(p->p_commons.table,
 						    cladatum->comkey);
 		if (!cladatum->comdatum) {
-			printf("security:  unknown common %s\n", cladatum->comkey);
+			DEBUG(__FUNCTION__, "unknown common %s\n",
+				cladatum->comkey);
 			goto bad;
 		}
 	}
@@ -1477,8 +1478,8 @@
 
 	if (strcmp(key, OBJECT_R) == 0) {
 		if (role->value != OBJECT_R_VAL) {
-			printf("Role %s has wrong value %d\n",
-			       OBJECT_R, role->value);
+			DEBUG(__FUNCTION__, "role %s has wrong value %d\n",
+				OBJECT_R, role->value);
 			role_destroy(key, role, NULL);
 			return -1;
 		}
@@ -1758,7 +1759,8 @@
                 for (genfs_p = NULL, genfs = p->genfs; genfs;
                      genfs_p = genfs, genfs = genfs->next) {
                         if (strcmp(newgenfs->fstype, genfs->fstype) == 0) {
-                                printf("security:  dup genfs fstype %s\n", newgenfs->fstype);
+				DEBUG(__FUNCTION__, "dup genfs fstype %s\n", 
+					newgenfs->fstype);
                                 goto bad;
                         }       
                         if (strcmp(newgenfs->fstype, genfs->fstype) < 0)
@@ -1801,8 +1803,11 @@
                         for (l = NULL, c = newgenfs->head; c;
                              l = c, c = c->next) {
                                 if (!strcmp(newc->u.name, c->u.name) &&
-                                    (!c->v.sclass || !newc->v.sclass || newc->v.sclass == c->v.sclass)) {
-                                        printf("security:  dup genfs entry (%s,%s)\n", newgenfs->fstype, c->u.name);
+                                    (!c->v.sclass || !newc->v.sclass || 
+				     newc->v.sclass == c->v.sclass)) {
+					DEBUG(__FUNCTION__, "dup genfs entry "
+						"(%s,%s)\n", newgenfs->fstype, 
+							c->u.name);
                                         goto bad;
                                 }
                                 len = strlen(newc->u.name);
@@ -1836,13 +1841,13 @@
 
 	buf = next_entry(fp, sizeof(uint32_t));
 	if (!buf) {
-		printf("security: mls: truncated level\n");
+		DEBUG(__FUNCTION__, "truncated level\n");
 		goto bad;
 	}
 	lp->sens = le32_to_cpu(buf[0]);
 
 	if (ebitmap_read(&lp->cat, fp)) {
-		printf("security: mls:  error reading level categories\n");
+		DEBUG(__FUNCTION__, "error reading level categories\n");
 		goto bad;
 	}
 	return 0;
@@ -2422,31 +2427,35 @@
                 target_str = POLICYDB_MOD_STRING;
         }
         else {
-                printf("security:  policydb magic number %#08x does not match expected magic number %#08x or %#08x\n",
+		DEBUG(__FUNCTION__, "policydb magic number %#08x does not "
+			"match expected magic number %#08x or %#08x\n",
 			buf[0], POLICYDB_MAGIC, POLICYDB_MOD_MAGIC);
 		return -1;
 	}
 
         len = buf[1];
         if (len != strlen(target_str)) {
-                printf("security:  policydb string length %zu does not match expected length %zu\n", len, strlen(target_str));
+		DEBUG(__FUNCTION__, "policydb string length %zu does not match "
+			"expected length %zu\n", len, strlen(target_str));
                 return -1;
         }
 
 	buf = next_entry(fp, len);
 	if (!buf) {
-		printf("security:  truncated policydb string identifier\n");
+		DEBUG(__FUNCTION__, "truncated policydb string identifier\n");
 		return -1;
 	}
 	policydb_str = malloc(len + 1);
 	if (!policydb_str) {
-		printf("security:  unable to allocate memory for policydb string of length %zu\n", len);
+		DEBUG(__FUNCTION__, "unable to allocate memory for policydb "
+			"string of length %zu\n", len);
 		return -1;
 	}
 	memcpy(policydb_str, buf, len);
 	policydb_str[len] = 0;
 	if (strcmp(policydb_str, target_str)) {
-		printf("security:  policydb string %s does not match my string %s\n", policydb_str, target_str);
+		DEBUG(__FUNCTION__, "policydb string %s does not match "
+			"my string %s\n", policydb_str, target_str);
 		free(policydb_str);
 		return -1;
 	}
@@ -2474,7 +2483,8 @@
 		   tells us which. */
 		policy_type = buf[bufindex];
                 if (policy_type != POLICY_MOD && policy_type != POLICY_BASE) {
-                        printf("Unknown module type: %#08x\n", policy_type);
+			DEBUG(__FUNCTION__, "unknown module type: %#08x\n",
+				policy_type);
                         return -1;
                 }
 		bufindex++;
@@ -2484,17 +2494,19 @@
         if (policy_type == POLICY_KERN || policy_type == POLICY_BASE) {
                 if (r_policyvers < POLICYDB_VERSION_MIN ||
                     r_policyvers > POLICYDB_VERSION_MAX) {
-                        printf("security:  policydb version %d does not match "
-                               "my version range %d-%d\n", buf[bufindex], POLICYDB_VERSION_MIN, POLICYDB_VERSION_MAX);
+			DEBUG(__FUNCTION__, "policydb version %d does not match "
+				"my version range %d-%d\n", buf[bufindex], 
+				POLICYDB_VERSION_MIN, POLICYDB_VERSION_MAX);
                         return -1;
                 }
         }
         else if (policy_type == POLICY_MOD) {
                 if (r_policyvers < MOD_POLICYDB_VERSION_MIN ||
                     r_policyvers > MOD_POLICYDB_VERSION_MAX) {
-                        printf("security:  policydb module version %d does not match "
-                               "my version range %d-%d\n", buf[bufindex],
-                               MOD_POLICYDB_VERSION_MIN, MOD_POLICYDB_VERSION_MAX);
+			DEBUG(__FUNCTION__, "policydb module version %d does "
+				"not match my version range %d-%d\n",
+				buf[bufindex], MOD_POLICYDB_VERSION_MIN,
+				MOD_POLICYDB_VERSION_MAX);
                         return -1;
                 }
         }
@@ -2515,13 +2527,15 @@
 
 	info = policydb_lookup_compat(r_policyvers, policy_type);
 	if (!info) {
-		printf("security:  unable to find policy compat info for version %d\n", r_policyvers);
+		DEBUG(__FUNCTION__, "unable to find policy compat info "
+			"for version %d\n", r_policyvers);
 		goto bad;
 	}
 
 	if (buf[bufindex] != info->sym_num || buf[bufindex + 1] != info->ocon_num) {
-		printf("security:  policydb table sizes (%d,%d) do not match mine (%d,%d)\n",
-		       buf[bufindex], buf[bufindex + 1], info->sym_num, info->ocon_num);
+		DEBUG(__FUNCTION__, "policydb table sizes (%d,%d) do not "
+			"match mine (%d,%d)\n", buf[bufindex], buf[bufindex + 1],
+			info->sym_num, info->ocon_num);
 		goto bad;
 	}
 
diff -Naur libsepol/src/private.h libsepol.new/src/private.h
--- libsepol/src/private.h	2005-08-21 12:56:17.000000000 -0400
+++ libsepol.new/src/private.h	2005-09-12 23:41:19.000000000 -0400
@@ -31,10 +31,6 @@
 };
 
 extern struct policydb_compat_info *policydb_lookup_compat(unsigned int version, unsigned int type);
-#ifdef __GNUC__
-__attribute__ ((format (printf, 1, 2)))
-#endif
-extern void __sepol_debug_printf(const char *fmt, ...);
 
 /* Reading from a policy "file". */
 static inline void *next_entry(struct policy_file * fp, size_t bytes)

Re: [ SEMANAGE ] Introduce record table

[Stephen Smalley]

On Mon, 2005-09-12 at 10:14 -0400, Ivan Gyurdiev wrote:
> The attached patch applies on top of the other one.
> It introduces the record table as the first thing
> which will be stored in the database config (a database config
> stores parameters specific to a database (a.k.a. file))
> 
> To clarify how this works - there's two sets of files here - one
> that works with generic record_t/record_key_t objects (treated as void*),
> and one that works with specific types that replace record_t/record_key_t.
> This essentially implements inheritance, which allows sharing of a lot of
> code (not yet merged). It's also a bit tricky to get it working, because we
> must keep track of where the specific types are defined.
> 
> The record table stores a set of functions which correspond
> to the record headers previously merged. It defines how certain
> operations are done for each particular type of record. In addition, it 
> provides
> some functions specific to the FILE backend. This table will
> be used by the file engine for polymorphism - it will work with
> the generic record types, but will be invoking the specific functions for
> each record based on what the table says.
> 
> Regarding database_init/close - those need to be integrated somewhere.. 
> likely
> on CONNECT/DISCONNECT...but I haven't done that yet..and it might change
> once semanage_handle_t appears.

(*key_free) prototype in src/record_file.h returns int, but the
implementations return void.  I assume void is correct.

Is there any concern about future namespace collision for the
dbase*/record* types, even though they are private to the library?  What
about when you start providing real implementations that have to bring
in external headers that may have conflicts?

-- 
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 ] Move more things to newer debug system

[Stephen Smalley]

On Mon, 2005-09-12 at 23:55 -0400, Ivan Gyurdiev wrote:
> The attached patch changes printf/fprintf usage in policydb.c, and
> avtab.c to the new debug system. This allows the user to set the print 
> function.
> It also writes to stderr by default, as opposed to stdout (which is 
> wrong, imho).
> As a side effect, this will append a more verbose prefix everywhere
> that gives the exact function name, rather than "security : mls" or 
> whatever.
> 
> This also removes the __sepol_debug_printf debug system from
> gen*, and changes genbools and genusers to use the newer system.
> 
> I haven't really tested this, but it's fairly straightforward, and I don't
> see what can break - it compiles without warnings.
> 
> ============
> We need to reduce the number of debug systems in the selinux libraries.
> printf's should be eradicated. There's also the alternative system that 
> semanage
> uses, based on buffering the errors - I'm not clear how that will
> be made to work in concert with sepol debugging.

We likely want to distinguish between debugging output (that should be
off by default) and error reporting output (that should always be
enabled).  Tresys had some concerns with the DEBUG() interface for
multi-threaded servers iirc; I've noted some concerns with their
write_error() interface in regards to have a uniform interface and
shared implementation and with the level of detail presently provided
via write_error().
  
-- 
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 ] Introduce record table

[Ivan Gyurdiev]

>(*key_free) prototype in src/record_file.h returns int, but the
>implementations return void.  I assume void is correct.
>  
>
void is correct - the free function should not fail.

>Is there any concern about future namespace collision for the
>dbase*/record* types, even though they are private to the library?  What
>about when you start providing real implementations that have to bring
>in external headers that may have conflicts?
>  
>
Yes, I am concerned about namespace collision, since I run into it all 
the time. However, I think it should be possible to eliminate it 
completely. In particular, specific record types should *never* be 
defined in headers. They're to be used in implementations only, and only 
in files that manipulate one type of record.

--
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 ] Move more things to newer debug system

[Ivan Gyurdiev]

>We likely want to distinguish between debugging output (that should be
>off by default) and error reporting output (that should always be
>enabled).  
>
Yes, we don't make that distiction currently. My patch treats all output 
as debugging. On the other hand, however, it leaves debugging as 
on-by-default for compatibility. I think we might not want any error 
reporting output in library code - libraries should use status codes to 
report the problem

>Tresys had some concerns with the DEBUG() interface for
>multi-threaded servers iirc.
>
>I've noted some concerns with their
>write_error() interface in regards to have a uniform interface and
>shared implementation and with the level of detail presently provided
>via write_error().
>  
>
I think there needs to be consistency across the library, whether it's 
DEBUG or write_error. The problem is that write_error requires 
significant modifications to APIs (user needs to pass error buffer to 
the library), whereas DEBUG does not, since it maintains the 
library-wide debugging. This is going to be a problem for the new 
semanage code I plan to put in as well...which currently uses DEBUG, 
which does not exist in semanage. If error buffers are required, I 
should alter the API to new functionality to support those. In any case, 
I think printfs are the wrong thing to do, and should be eliminated from 
sepol.

--
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 ] Introduce record table

[Ivan Gyurdiev]

>> Is there any concern about future namespace collision for the
>> dbase*/record* types, even though they are private to the library?  What
>> about when you start providing real implementations that have to bring
>> in external headers that may have conflicts?
>>  
>>
> Yes, I am concerned about namespace collision, since I run into it all 
> the time. However, I think it should be possible to eliminate it 
> completely. In particular, specific record types should *never* be 
> defined in headers. They're to be used in implementations only, and 
> only in files that manipulate one type of record.

Actually I misunderstood what you were saying....you're concerned about 
the dbase_* record_* prefix?
That's easy enough to change, if it's a problem, but I don't plan on 
bringing in any extrnal headers other than the standard C library. 
Besides, they're internal, so they can always be changed if a problem 
comes up. Suggest another prefix, and I'll get it changed...

I was talking about collision between specific record types and void* 
record_t above. Essentially you can't use both..ever. Either you're 
going to do polymorphism, and not consider specific types equivalent to 
record_t... or you're doing implementation of a single record (and you 
can't mix different types as record_t). That's a bit ugly, but I don't 
think it will be a problem...


--
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 ] Move more things to newer debug system

[Joshua Brindle]

On Tue, 2005-09-13 at 18:26 -0400, Ivan Gyurdiev wrote:
> >We likely want to distinguish between debugging output (that should be
> >off by default) and error reporting output (that should always be
> >enabled).  
> >
> Yes, we don't make that distiction currently. My patch treats all output 
> as debugging. On the other hand, however, it leaves debugging as 
> on-by-default for compatibility. I think we might not want any error 
> reporting output in library code - libraries should use status codes to 
> report the problem
> 
> >Tresys had some concerns with the DEBUG() interface for
> >multi-threaded servers iirc.
> >
> >I've noted some concerns with their
> >write_error() interface in regards to have a uniform interface and
> >shared implementation and with the level of detail presently provided
> >via write_error().
> >  
> >
> I think there needs to be consistency across the library, whether it's 
> DEBUG or write_error. The problem is that write_error requires 
> significant modifications to APIs (user needs to pass error buffer to 
> the library), whereas DEBUG does not, since it maintains the 
> library-wide debugging. This is going to be a problem for the new 
> semanage code I plan to put in as well...which currently uses DEBUG, 
> which does not exist in semanage. If error buffers are required, I 
> should alter the API to new functionality to support those. In any case, 
> I think printfs are the wrong thing to do, and should be eliminated from 
> sepol.
> 

Yes, libraries should not be outputting error conditions to the stdout
or stderr. They should pass error strings and values to the frontend
apps. 

The write_error is to pass an _error condition_ to the front end, this
has nothing to do with debugging. Personally, I don't know what the
value of a specialized debugging system in the library is when gdb and
integrated debuggers are so good, but clearly handling errors and
passing descriptive error messages to the frontend are necessary.

If you have debug on-by-default it seems like you are really doing error
handling. If this is the case we may need to consolidate these systems
but error handling and debugging are hardly the same things.

iirc, as Steve mentioned, there are some thread safety issues with DEBUG
and since both the userspace security server and the policy management
server are threaded this could be problematic. 

DEBUG also passes the function name, which may be useful for debugging,
but is hardly useful for error reporting. 

The error system should be about passing descriptive messages to the
caller and this needs to be done by passing a buffer to the functions
that may return an error, not be setting a function pointer to an
arbitrary function in the frontend to handle error conditions.

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.

Re: [ SEPOL ] Move more things to newer debug system

[Ivan Gyurdiev]

>If you have debug on-by-default it seems like you are really doing error
>handling. If this is the case we may need to consolidate these systems
>but error handling and debugging are hardly the same things.
>  
>
Yes, I agree here - the DEBUG system is really doing error handling...
it detects an error condition, and informs the caller of what went wrong 
at every level of
the function stack (at least that's how I use it in the user and port 
functions). It has no effect
if there is no error condition. Perhaps it's a misnomer.

>iirc, as Steve mentioned, there are some thread safety issues with DEBUG
>and since both the userspace security server and the policy management
>server are threaded this could be problematic. 
>  
>
The issues are not with DEBUG - they're with the general principle
of library-wide debugging/error-messaging vs per-function error handling 
(state object
passed by caller).

>The error system should be about passing descriptive messages to the
>caller and this needs to be done by passing a buffer to the functions
>that may return an error,
>
How do we pass such a buffer without rewriting the libsepol API?
DEBUG has the advantage of being backwards compatible.

> not be setting a function pointer to an
>arbitrary function in the frontend to handle error conditions.
>  
>
That's an implementation detail that you don't like, because of the 
semanage server.
It's not the core issue here - you could have the caller pass a callback 
with argument (for thread id)
to be invoked on error just as easily as passing in a buffer.

The core of the problem is library-wide debugging without 
caller-supplied state object.

--
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 ] Move more things to newer debug system

[Ivan Gyurdiev]

 > The core of the problem is library-wide debugging without 
caller-supplied state object.
 
By the way, the same thing applies to all those functions that use a 
library-wide policydb.
I think that's a mistake, and they should take the policydb as an 
argument. Global variables
are evil and not thread-friendly.

--
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 ] Move more things to newer debug system

[Luke Kenneth Casson Leighton]

On Tue, Sep 13, 2005 at 11:33:04PM -0400, Ivan Gyurdiev wrote:
> >The error system should be about passing descriptive messages to the
> >caller and this needs to be done by passing a buffer to the functions
> >that may return an error,
> >
> How do we pass such a buffer without rewriting the libsepol API?
> DEBUG has the advantage of being backwards compatible.
 
 okay.

 i refrained from replying regarding library design, which i
 learned about from an expert - andrew tridgell.  i refrained
 because i believed i would be seen as attemtping to teach
 people who already knew about these matters.
 
 given that there have now been at least three messages bouncing
 back-and-forth indicating that there have been some quite
 serious basic design mistakes (global variables, printfs for debug
 reporting etc.), i thought it best to chip in.

 properly libraries should never call _any_ functions that it has not
 explicitly been given access to.

 if there are lots of functions it needs, then the library should be
 passed in an array (vector table) of those functions.

 the best designed libraries should have ONE external function
 and ZERO global variables.  the external function should
 be setup function which returns a vector table pointing to
 static internal functions.

 one of those static internal functions should be an "initialisation"
 function, via which the library should RECEIVE a vector table pointing
 to functions that it needs to make use of - the most obvious ones being
 of course malloc, free and realloc.


 stuff backwards compatibility - royally.
 
 design the library properly.

 pass in a pointer to a memory free and a memory alloc function.

 pass in a debug reporting function, with clear instructions
 that the memory allocated to report the error should be freed
 by the caller (or some other technique e.g. a negotiation
 technique to request a buffer for error reporting of minimum
 size X, which could be static).

 
 the extreme case of library development is kernel programming,
 especially in Windows NT's design, for example the LSA -
 local security authority.  mostly, in NT, things are designed
 the way they are because MS can't make up their minds as
 to whether to have code in kernel or in userspace, so they
 design everything for kernelspace, just in case.

 anyway.


 i strongly advise you to consider _not_ putting backwards
 compatibility as a high priority over good library design.

 and to treat the library as if it was to be put into the
 linux kernel.

 l.


--
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 ] Move more things to newer debug system

[Luke Kenneth Casson Leighton]

On Tue, Sep 13, 2005 at 11:33:04PM -0400, Ivan Gyurdiev wrote:
> The core of the problem is library-wide debugging without 
> caller-supplied state object.
 
 why would you not wish to do that?

 is there a specific reason which _prevents_ caller-supplied state?
 (which is how the best libraries are designed)

 l.

--
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 ] Move more things to newer debug system

[Stephen Smalley]

On Wed, 2005-09-14 at 08:00 +0100, Luke Kenneth Casson Leighton wrote:
>  given that there have now been at least three messages bouncing
>  back-and-forth indicating that there have been some quite
>  serious basic design mistakes (global variables, printfs for debug
>  reporting etc.), i thought it best to chip in.

Hi,

Point of clarification:  libsepol was originally just pulled out of the
checkpolicy program, so these characteristics reflect its origins as a
single monolithic program.   

>  design the library properly.

By all means, do things right going forward, but don't overengineer a
library that was only ever intended to share some implementation details
previously hidden within checkpolicy with some other users, and that is
going back toward being a static library only for a very small set of
users.

-- 
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 ] Move more things to newer debug system

[Stephen Smalley]

On Tue, 2005-09-13 at 18:26 -0400, Ivan Gyurdiev wrote:
> Yes, we don't make that distiction currently. My patch treats all output 
> as debugging. On the other hand, however, it leaves debugging as 
> on-by-default for compatibility. I think we might not want any error 
> reporting output in library code - libraries should use status codes to 
> report the problem

Status codes aren't sufficiently informative.  

> I think there needs to be consistency across the library, whether it's 
> DEBUG or write_error. The problem is that write_error requires 
> significant modifications to APIs (user needs to pass error buffer to 
> the library), whereas DEBUG does not, since it maintains the 
> library-wide debugging. This is going to be a problem for the new 
> semanage code I plan to put in as well...which currently uses DEBUG, 
> which does not exist in semanage. If error buffers are required, I 
> should alter the API to new functionality to support those. In any case, 
> I think printfs are the wrong thing to do, and should be eliminated from 
> sepol.

Yes, the printfs are just a legacy of its origins from checkpolicy (and
in fact, from its origins as shared code with the kernel security
server, where printf was remapped to printk(KERN_WARNING...)).

-- 
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 ] Move more things to newer debug system

[Stephen Smalley]

On Tue, 2005-09-13 at 23:33 -0400, Ivan Gyurdiev wrote:
> Yes, I agree here - the DEBUG system is really doing error handling...
> it detects an error condition, and informs the caller of what went wrong 
> at every level of
> the function stack (at least that's how I use it in the user and port 
> functions). It has no effect
> if there is no error condition. Perhaps it's a misnomer.

To be precise, DEBUG() is providing supplemental information when an
error occurs to help in debugging the underlying cause.  The error
handling itself consists of just returning an error code up the call
chain (although they aren't as informative as one might like).
write_error() is likewise providing supplemental information when an
error occurs, but is focused on just reporting informative error
messages to the user rather than helping with debugging the underlying
cause or the precise point of failure.

write_error() is also limited to a single error message, whereas DEBUG()
can emit any number of messages during a single operation.  This shows
up particularly in the assertion checking code where we'd like (at least
for checkpolicy) to report all assertion violations in one pass.
Similarly, it would be nice if the hieararchy checking code could report
all violations in one pass (and if it provided more detail about the
actual cause of the violation).

-- 
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 ] Move more things to newer debug system

[Stephen Smalley]

On Tue, 2005-09-13 at 23:37 -0400, Ivan Gyurdiev wrote:
>  > The core of the problem is library-wide debugging without 
> caller-supplied state object.
>  
> By the way, the same thing applies to all those functions that use a 
> library-wide policydb.
> I think that's a mistake, and they should take the policydb as an 
> argument. Global variables
> are evil and not thread-friendly.

Shrug.  In the kernel security server, from which this code originated,
there is a single active policydb and sidtab at a time, and the service
functions operate on them.  I'm dubious that one truly needs per-thread
policydb/sidtab instances in the userspace security server.  In any
event, as noted repeatedly, libsepol is just a blob of code taken from
checkpolicy much of which was taken from the kernel security server, so
let's get over its deficiencies and move on.  

As for threads, Alan Cox said it best:  "A computer is a state machine.
Threads are for people who cant program state machines.".

-- 
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 ] Move more things to newer debug system

[Joshua Brindle]

Stephen Smalley wrote:

>On Tue, 2005-09-13 at 23:33 -0400, Ivan Gyurdiev wrote:
>  
>
>>Yes, I agree here - the DEBUG system is really doing error handling...
>>it detects an error condition, and informs the caller of what went wrong 
>>at every level of
>>the function stack (at least that's how I use it in the user and port 
>>functions). It has no effect
>>if there is no error condition. Perhaps it's a misnomer.
>>    
>>
>
>To be precise, DEBUG() is providing supplemental information when an
>error occurs to help in debugging the underlying cause.  The error
>handling itself consists of just returning an error code up the call
>chain (although they aren't as informative as one might like).
>write_error() is likewise providing supplemental information when an
>error occurs, but is focused on just reporting informative error
>messages to the user rather than helping with debugging the underlying
>cause or the precise point of failure.
>
>  
>
Yes, and I think we all agree that we need better error reporting to the 
calling functions, we particularly need this in things like linking and 
expanding where alot of stuff happens deep in libsepol and there are 
many possible error, conditions, many of which the user needs to know 
about in order to fix the policy (dependancy problems, etc)

>write_error() is also limited to a single error message, whereas DEBUG()
>can emit any number of messages during a single operation.  This shows
>up particularly in the assertion checking code where we'd like (at least
>for checkpolicy) to report all assertion violations in one pass.
>Similarly, it would be nice if the hieararchy checking code could report
>all violations in one pass (and if it provided more detail about the
>actual cause of the violation).
>  
>

Fair enough, for things like assertion and hierarchy checking we can 
pass a function pointer in and use the same basic system without the 
global DEBUG function pointer and still get what we want there. If we 
tried to use DEBUG for error reporting (as I think Ivan is suggesting) 
the calling function wouldn't get an error code, and thus check it's own 
error state until the first call in the chain and could potentially lose 
information. Having the specific error from the lowest point of failure 
seems much more useful than getting a less and less specific error from 
each funtion while propagating error codes.

I know modifying every function in sepol to use a write_error like 
system is undesirable but libsemanage already has write_error used 
exclusively so going to DEBUG with it also seems undesirable. 
libsemanage already passes a handle which contains an error buffer which 
gives us thread safety and the ability to easily return errors in a way 
the client app can use them (semodule will probably just print them out, 
whereas the policy server will have to do more).

--
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 ] Move more things to newer debug system

[Stephen Smalley]

On Wed, 2005-09-14 at 09:21 -0400, Joshua Brindle wrote:
> Fair enough, for things like assertion and hierarchy checking we can 
> pass a function pointer in and use the same basic system without the 
> global DEBUG function pointer and still get what we want there. If we 
> tried to use DEBUG for error reporting (as I think Ivan is suggesting) 
> the calling function wouldn't get an error code, and thus check it's own 
> error state until the first call in the chain and could potentially lose 
> information. Having the specific error from the lowest point of failure 
> seems much more useful than getting a less and less specific error from 
> each funtion while propagating error codes.

I'm not sure I follow.  As currently used, I think a call to DEBUG() is
always followed by returning an error code (although there might
possibly be multiple calls to DEBUG() prior to returning, e.g. if we use
it in the assertion checking code in place of the fprintf), although
those error codes need to be more specific as well.

> I know modifying every function in sepol to use a write_error like 
> system is undesirable but libsemanage already has write_error used 
> exclusively so going to DEBUG with it also seems undesirable. 
> libsemanage already passes a handle which contains an error buffer which 
> gives us thread safety and the ability to easily return errors in a way 
> the client app can use them (semodule will probably just print them out, 
> whereas the policy server will have to do more).
-- 
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 ] Move more things to newer debug system

[Dale Amon]

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

On Wed, Sep 14, 2005 at 09:16:19AM -0400, Stephen Smalley wrote:
> As for threads, Alan Cox said it best:  "A computer is a state machine.
> Threads are for people who cant program state machines.".

A true statement, but... complexity explosion makes
it useful to move to techniques which make 'what is
happening' clearer and easier to understand and to
verify.

-- 
------------------------------------------------------
   Dale Amon     amon@islandone.org    +44-7802-188325
       International linux systems consultancy
     Hardware & software system design, security
    and networking, systems programming and Admin
	      "Have Laptop, Will Travel"
------------------------------------------------------

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [ SEPOL ] Move more things to newer debug system

[Joshua Brindle]

Stephen Smalley wrote:

>On Wed, 2005-09-14 at 09:21 -0400, Joshua Brindle wrote:
>  
>
>>Fair enough, for things like assertion and hierarchy checking we can 
>>pass a function pointer in and use the same basic system without the 
>>global DEBUG function pointer and still get what we want there. If we 
>>tried to use DEBUG for error reporting (as I think Ivan is suggesting) 
>>the calling function wouldn't get an error code, and thus check it's own 
>>error state until the first call in the chain and could potentially lose 
>>information. Having the specific error from the lowest point of failure 
>>seems much more useful than getting a less and less specific error from 
>>each funtion while propagating error codes.
>>    
>>
>
>I'm not sure I follow.  As currently used, I think a call to DEBUG() is
>always followed by returning an error code (although there might
>possibly be multiple calls to DEBUG() prior to returning, e.g. if we use
>it in the assertion checking code in place of the fprintf), although
>those error codes need to be more specific as well.
>
>  
>
What I mean is, if there are multiple calls to DEBUG prior to returning 
to the originating function then there are multiple error messages of 
varying specificity sitting around somewhere, assuming we even store 
them all and not just the last one. This isn't ideal for error 
reporting, but it is for debugging. Personally, again, I feel that gdb 
is far superior to any kind of debug system you can put in a library so 
I'd rather focus on proper error reporting, which DEBUG doesn't do in a 
reasonable way.

>>I know modifying every function in sepol to use a write_error like 
>>system is undesirable but libsemanage already has write_error used 
>>exclusively so going to DEBUG with it also seems undesirable. 
>>libsemanage already passes a handle which contains an error buffer which 
>>gives us thread safety and the ability to easily return errors in a way 
>>the client app can use them (semodule will probably just print them out, 
>>whereas the policy server will have to do more).
>>    
>>


--
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 ] Move more things to newer debug system

[Stephen Smalley]

On Wed, 2005-09-14 at 10:45 -0400, Joshua Brindle wrote:
> What I mean is, if there are multiple calls to DEBUG prior to returning 
> to the originating function then there are multiple error messages of 
> varying specificity sitting around somewhere, assuming we even store 
> them all and not just the last one. This isn't ideal for error 
> reporting, but it is for debugging.

Even in the error reporting case, allowing for multiple messages (e.g.
for multiple assertion failures, hierarchy failures, etc) is useful.
Those would be of comparable specificity rather than varying degrees.

If omitting the traceback data is your primary concern, possibly it
could be explicitly identified in some manner in the DEBUG() interface
so that the callback can easily drop it for error reporting purposes.

>  Personally, again, I feel that gdb 
> is far superior to any kind of debug system you can put in a library so 
> I'd rather focus on proper error reporting, which DEBUG doesn't do in a 
> reasonable way.

Requiring people to re-run the offending code through a debugger
significantly reduces your ability to get useful bug reports...

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

info on SELinux support for IPSEC

[Prakash Saivasan]

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


Hi All,

    Can anyone provide me with information on SELinux support/integration with IPSEC ? Is there any document that talks about the implementation details policy et al.,

 

Thanking you

Prakash S



It may be bad manners to talk with your mouth full, but it isn't too good either if you speak when your head is empty.
                                                                   !!!!Jaihind !!!!

 


 

Send instant messages to your online friends http://in.messenger.yahoo.com 

[-- Attachment #2: Type: text/html, Size: 915 bytes --]

Re: [ SEPOL ] Move more things to newer debug system

[Joshua Brindle]

Stephen Smalley wrote:

>On Wed, 2005-09-14 at 10:45 -0400, Joshua Brindle wrote:
>  
>
>>What I mean is, if there are multiple calls to DEBUG prior to returning 
>>to the originating function then there are multiple error messages of 
>>varying specificity sitting around somewhere, assuming we even store 
>>them all and not just the last one. This isn't ideal for error 
>>reporting, but it is for debugging.
>>    
>>
>
>Even in the error reporting case, allowing for multiple messages (e.g.
>for multiple assertion failures, hierarchy failures, etc) is useful.
>Those would be of comparable specificity rather than varying degrees.
>
>  
>
yes, in those two special cases reporting multiple messages is useful, 
we may need to handle them seperately. There is nothing preventing 
multiple error messages with write_error, we could even add append_error 
and have multiple errors in the same buffer. Granted that, at least with 
assertion failures, the buffer wouldn't be large enough to hold all the 
errors.

>If omitting the traceback data is your primary concern, possibly it
>could be explicitly identified in some manner in the DEBUG() interface
>so that the callback can easily drop it for error reporting purposes.
>
>  
>
Omitting it isn't my concern if it is really for debugging, I do not 
think that we should try to use the debug system for error reporting to 
the originating function if it is going to be a debug system.

>> Personally, again, I feel that gdb 
>>is far superior to any kind of debug system you can put in a library so 
>>I'd rather focus on proper error reporting, which DEBUG doesn't do in a 
>>reasonable way.
>>    
>>
>
>Requiring people to re-run the offending code through a debugger
>significantly reduces your ability to get useful bug reports...
>
>  
>
Fair enough, I had only thought about the developer case, not the user 
case. It seems like the debug system is useful, especially in this case, 
for debugging, but not particularly for error reporting. If Ivan wants 
to put DEBUG in libsemanage we are ok with that, assuming that it is 
used for debugging and corresponding error reporting is also added for 
errors. Since the policy server will be a primary user of libsemanage, 
and is threaded we really need the error reporting in the connection 
handle to report to the user on the other end, but debugging can't hurt.

As for libsepol, I don't think we should go down this road of using 
DEBUG for error reporting. Since parts of libsepol aren't threadsafe 
already (eg., reading, writing) we can use a global error buffer for 
error reporting. If the need for thread safety ever comes up we could 
fix up the API at that point but, as you said before, libsepol is static 
with few users, so API changes aren't a dramatic thing.

In short, it is apparent that we need both debug and write_error, for 
different reasons, and I see no reason why we can't use both. DEBUG, 
then, could be used more liberally for general library information since 
it would presumably default to off.


--
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 ] Move more things to newer debug system

[Stephen Smalley]

On Wed, 2005-09-14 at 11:33 -0400, Joshua Brindle wrote:
> Omitting it isn't my concern if it is really for debugging, I do not 
> think that we should try to use the debug system for error reporting to 
> the originating function if it is going to be a debug system.

I was thinking that unifying them would be ideal, and the callbacks can
decide how they want to filter the results.  Just requires that the
callbacks be provided with a level indicator in addition to the other
arguments that indicates whether it is debug info only or error info.
Not unlike syslog(3) or printk(9).

-- 
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 ] Stub out user/port functionality

[Stephen Smalley]

On Mon, 2005-09-12 at 08:06 -0400, Ivan Gyurdiev wrote:
> The attached patch for libsemanage stubs out the functionality for managing
> user and port records. This means simply editing the config files...
> As discussed with Joshua, loading the users and ports into policy needs
> to be accomplished at commit time, after linking in modules.
> 
> I'm still not entirely clear how all the pieces will fit together, but
> I think it would be good to merge a stubbed-out skeleton...later we could
> change it to make use of the planned semanage_handle_t.
> 
> The second patch fixes sepol headers to include stddef.h whenever size_t 
> is used.

Thanks, merged upstream.

-- 
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 ] Introduce record table

[Stephen Smalley]

On Mon, 2005-09-12 at 10:14 -0400, Ivan Gyurdiev wrote:
> The attached patch applies on top of the other one.
> It introduces the record table as the first thing
> which will be stored in the database config (a database config
> stores parameters specific to a database (a.k.a. file))

Thanks, merged upstream.

-- 
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 ] Move more things to newer debug system

[Stephen Smalley]

On Mon, 2005-09-12 at 23:55 -0400, Ivan Gyurdiev wrote:
> The attached patch changes printf/fprintf usage in policydb.c, and
> avtab.c to the new debug system. This allows the user to set the print 
> function.
> It also writes to stderr by default, as opposed to stdout (which is 
> wrong, imho).
> As a side effect, this will append a more verbose prefix everywhere
> that gives the exact function name, rather than "security : mls" or 
> whatever.
> 
> This also removes the __sepol_debug_printf debug system from
> gen*, and changes genbools and genusers to use the newer system.
> 
> I haven't really tested this, but it's fairly straightforward, and I don't
> see what can break - it compiles without warnings.

Thanks, merged upstream.

-- 
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 ] Move more things to newer debug system

[Joshua Brindle]

Stephen Smalley wrote:

>On Wed, 2005-09-14 at 11:33 -0400, Joshua Brindle wrote:
>  
>
>>Omitting it isn't my concern if it is really for debugging, I do not 
>>think that we should try to use the debug system for error reporting to 
>>the originating function if it is going to be a debug system.
>>    
>>
>
>I was thinking that unifying them would be ideal, and the callbacks can
>decide how they want to filter the results.  Just requires that the
>callbacks be provided with a level indicator in addition to the other
>arguments that indicates whether it is debug info only or error info.
>Not unlike syslog(3) or printk(9).
>  
>
So, do you mean taking write_error, optionally passing in a string and 
size, having it write to the string if it's present and then calling the 
debug function pointer if it is present? That way we'll get the error in 
the handle so that the caller can deal with it and the debug function 
would also be called. Then the caller could choose to use debug by 
setting the pointer and other apps that need an error in the handle 
could use that?

This seems reasonable to me.. Ivan?

--
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 ] Move more things to newer debug system

[Stephen Smalley]

On Wed, 2005-09-14 at 12:06 -0400, Joshua Brindle wrote:
> So, do you mean taking write_error, optionally passing in a string and 
> size, having it write to the string if it's present and then calling the 
> debug function pointer if it is present? That way we'll get the error in 
> the handle so that the caller can deal with it and the debug function 
> would also be called. Then the caller could choose to use debug by 
> setting the pointer and other apps that need an error in the handle 
> could use that?

I think you'd still want a level/priority value to distinguish debug
from error messages, with only the latter being written to the buffer
and all of them being fed to the debug callback if it is defined.  

If using write_error, it needs a standard interface defined in a
single .h file with a single shared implementation in a .c file in
libsepol and libsemanage, rather than multiple variants spread around.
No private state data types passed to it, just the common error
buffer/size arguments.

> This seems reasonable to me.. Ivan?

BTW, I just noticed - aren't the calls to sepol_debug() by
sepol_enable/disable_debug() broken?  If you just set the function
pointer, you don't want to clobber it using the old interface.

-- 
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 ] Move more things to newer debug system

[Ivan Gyurdiev]

>BTW, I just noticed - aren't the calls to sepol_debug() by
>sepol_enable/disable_debug() broken?  If you just set the function
>pointer, you don't want to clobber it using the old interface.
>
>  
>
The old interface is supposed to go away, but until it does, the new 
interface must provide the same functionality, which is why it maintains 
printing to stdout using the old system. By the way, this is *yet 
another* debug system - __sepol_debug_printf...

I'll respond to the rest of the discussion later tonight...







--
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 ] Move more things to newer debug system

[Ivan Gyurdiev]

Ivan Gyurdiev wrote:

>
>> BTW, I just noticed - aren't the calls to sepol_debug() by
>> sepol_enable/disable_debug() broken?  If you just set the function
>> pointer, you don't want to clobber it using the old interface.
>>
>>  
>>
> The old interface is supposed to go away, but until it does, the new 
> interface must provide the same functionality, which is why it 
> maintains printing to stdout using the old system. By the way, this is 
> *yet another* debug system - __sepol_debug_printf...
>
> I'll respond to the rest of the discussion later tonight...
>
Actually you're right - the compat call should be removed...

--
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 ] Move more things to newer debug system

[Stephen Smalley]

On Wed, 2005-09-14 at 15:05 +0100, Dale Amon wrote:
> On Wed, Sep 14, 2005 at 09:16:19AM -0400, Stephen Smalley wrote:
> > As for threads, Alan Cox said it best:  "A computer is a state machine.
> > Threads are for people who cant program state machines.".
> 
> A true statement, but... complexity explosion makes
> it useful to move to techniques which make 'what is
> happening' clearer and easier to understand and to
> verify.

Sorry, that isn't a motivation to move to threads.  Quite the opposite.

-- 
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: info on SELinux support for IPSEC

[Stephen Smalley]

On Wed, 2005-09-14 at 16:26 +0100, Prakash Saivasan wrote:
> Hi All,
> 
>     Can anyone provide me with information on SELinux
> support/integration with IPSEC ? Is there any document that talks
> about the implementation details policy et al.,

The necessary kernel patches are still being discussed on the netdev
mailing list.  An updated patch for ipsec tools has been posted recently
to the redhat-lspp list.  Nothing upstream yet.

A presentation from last year's SELinux Symposium describes the overall
approach:
http://www.selinux-symposium.org/2005/presentations/session2/2-3-jaeger.pdf

It is also described in the original submissions on netdev:
http://marc.theaimsgroup.com/?l=linux-netdev&m=111629897618372&w=2
http://marc.theaimsgroup.com/?l=linux-netdev&m=111629897610638&w=2

-- 
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 ] Move more things to newer debug system

[Stephen Smalley]

On Wed, 2005-09-14 at 13:16 -0400, Ivan Gyurdiev wrote:
> The old interface is supposed to go away, but until it does, the new 
> interface must provide the same functionality, which is why it maintains 
> printing to stdout using the old system. By the way, this is *yet 
> another* debug system - __sepol_debug_printf...

Well, it was another debug system (introduced by Dan earlier for
genbools/genusers), but your recent patches replaced all occurrences
AFAICS.

What remains are:
- the DEBUG(<function name>, <fmt>, ...) interface you introduced,
- the write_error(<state == buffer, size>, <fmt>, ...) interface Tresys
introduced with the module infrastructure,
- direct snprintf calls on error buffer parameters (hierarchy.c,
module.c) in the hierarchy checking and module infrastructure,
- if (state->verbose) printf calls (expand.c, link.c) for debugging in
the module infrastructure, 
- lingering printf/fprintf occurrences, most of which are from the
original inherited checkpolicy/security server code.

-- 
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 ] Move more things to newer debug system

[Ivan Gyurdiev]

> So, do you mean taking write_error, optionally passing in a string and 
> size, having it write to the string if it's present and then calling 
> the debug function pointer if it is present? That way we'll get the 
> error in the handle so that the caller can deal with it and the debug 
> function would also be called. Then the caller could choose to use 
> debug by setting the pointer and other apps that need an error in the 
> handle could use that?
>
> This seems reasonable to me.. Ivan?

So, I'm not clear on the exact migration steps that need to be taken here.
I've just changed a lot of printfs to DEBUG calls. I'm not entirely sure 
whether
they qualify as error reporting or debugging (probably the former). In 
fact,
the distinction is still a bit lost to me - it seems as if all current 
uses of DEBUG
are for error handling, and none are for debugging. Perhaps you could 
give an
example to clarify the situation...

Next, what to do we do with all those printf calls that remain, and the ones
that are now DEBUG calls.... if they are in fact error-reporting statements,
then I understand you want them written in an error buffer. Where does
the buffer come from, given the current API?

I am in favor of moving all messaging systems into one .c and .h file,
and making things more consistent.  I also like the idea of output
levels for debugging and error-messaging, although I'm not entirely sure
where I would use which.


--
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 ] Move more things to newer debug system

[Stephen Smalley]

On Wed, 2005-09-14 at 15:37 -0400, Ivan Gyurdiev wrote:
> So, I'm not clear on the exact migration steps that need to be taken here.
> I've just changed a lot of printfs to DEBUG calls. I'm not entirely sure 
> whether
> they qualify as error reporting or debugging (probably the former).

Yes, that does seem to be the common case.

>  In 
> fact,
> the distinction is still a bit lost to me - it seems as if all current 
> uses of DEBUG
> are for error handling, and none are for debugging. Perhaps you could 
> give an
> example to clarify the situation...

Offhand, I don't know of an actual debugging user of DEBUG() yet,
although it seems that the if (state->verbose) printf statements in the
Tresys code would qualify as potential debugging users.  There are also
"informational" cases, like the printfs in policydb_index_others.
syslog(3) does have distinct notions of debug, info, warning, error,
critical, alert, and emergency as its priority values.

> Next, what to do we do with all those printf calls that remain, and the ones
> that are now DEBUG calls.... if they are in fact error-reporting statements,
> then I understand you want them written in an error buffer. Where does
> the buffer come from, given the current API?

What particular interfaces are an issue here?  Interfaces that are only
exported by the static library can be modified with minimal impact.
sepol_genusers/genbools are going to be obsoleted by the shift to
regenerating the kernel binary policy file on each update to booleans or
users.  

In the interim, write_error can allow for the buffer to be NULL, in
which case it will only invoke the callback (if it is set) with the
other arguments as inputs.  If the buffer is non-NULL _and_ the
priority/level is >= the error level, then it will write the formatted
error message to the buffer and then invoke the callback (if it is set)
with the other arguments as inputs.  Hence, sepol_genusers/genbools can
continue to emit errors via the callback for now.

> I am in favor of moving all messaging systems into one .c and .h file,
> and making things more consistent.  I also like the idea of output
> levels for debugging and error-messaging, although I'm not entirely sure
> where I would use which.

-- 
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 ] Move more things to newer debug system

[Ivan Gyurdiev]

> What I mean is, if there are multiple calls to DEBUG prior to 
> returning to the originating function then there are multiple error 
> messages of varying specificity sitting around somewhere, assuming we 
> even store them all and not just the last one. This isn't ideal for 
> error reporting, but it is for debugging. 

This isn't as clear-cut as it appears. It was never my intent to print 
the call stack for the purposes of debugging. That's why I say DEBUG is 
a misnomer. The reason I print the call stack at every level is exactly 
because you have different levels of specifity, which provide different 
kinds of information, and not always redundancy. The lowest level often 
doesn't make any sense at all, because it is too low-level. The levels 
above it provide context information as to what's going on. The callee 
by itself has no clue of the caller's intent. The caller has doesn't 
understand what the exact error was - they work together. This will 
become more clear with the record engine, which relies on polymorphism - 
the engine would not have a lot of information about what's going on 
with the particular record types. An individual handler working on a 
record would not know what the overall task/query was... a parsing 
helper would know exactly where the invalid input was, but wouldn't know 
what it represents, or why it's important.

That said, I agree that the end result isn't particularly user-friendly. 
It looks similar to what ALSA lib does
when it crashes in mplayer. I'm not entirely sure what should be done to 
fix it.

--
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 ] Move more things to newer debug system

[Stephen Smalley]

On Wed, 2005-09-14 at 15:50 -0400, Stephen Smalley wrote:
> On Wed, 2005-09-14 at 15:37 -0400, Ivan Gyurdiev wrote:
> >  In 
> > fact,
> > the distinction is still a bit lost to me - it seems as if all current 
> > uses of DEBUG
> > are for error handling, and none are for debugging. Perhaps you could 
> > give an
> > example to clarify the situation...
> 
> Offhand, I don't know of an actual debugging user of DEBUG() yet,

Ah, sorry.  I think Joshua wanted to omit the function name and
traceback info from the error message saved in the buffer for return to
the client/user.  Which would mean that only the deepest level DEBUG()
calls would qualify as "errors" in his thinking.  But you just noted in
your other reply that this doesn't work very well.

-- 
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 ] Move more things to newer debug system

[Ivan Gyurdiev]

>>Next, what to do we do with all those printf calls that remain, and the ones
>>that are now DEBUG calls.... if they are in fact error-reporting statements,
>>then I understand you want them written in an error buffer. Where does
>>the buffer come from, given the current API?
>>    
>>
>
>What particular interfaces are an issue here? 
>
All of them? For example, the policydb.c code just changed to use DEBUG...
policydb_read()
policydb_write()
everything else...


--
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 ] Move more things to newer debug system

[Dale Amon]

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

On Wed, Sep 14, 2005 at 02:07:23PM -0400, Stephen Smalley wrote:
> > > As for threads, Alan Cox said it best:  "A computer is a state machine.
> > > Threads are for people who cant program state machines.".
> > 
> > A true statement, but... complexity explosion makes
> > it useful to move to techniques which make 'what is
> > happening' clearer and easier to understand and to
> > verify.
> 
> Sorry, that isn't a motivation to move to threads.  Quite the opposite.

I'm not talking about selinux needs per-se... but I
have found it much clearer to deal with multiple events
via multiple processes (same thing in linux)  rather 
than with a state machine to try to handle them all. 
I use both techniques all the time. 

-- 
------------------------------------------------------
   Dale Amon     amon@islandone.org    +44-7802-188325
       International linux systems consultancy
     Hardware & software system design, security
    and networking, systems programming and Admin
	      "Have Laptop, Will Travel"
------------------------------------------------------

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [ SEPOL ] Move more things to newer debug system

[Ivan Gyurdiev]

Ivan Gyurdiev wrote:

>
>>> Next, what to do we do with all those printf calls that remain, and 
>>> the ones
>>> that are now DEBUG calls.... if they are in fact error-reporting 
>>> statements,
>>> then I understand you want them written in an error buffer. Where does
>>> the buffer come from, given the current API?
>>>   
>>
>>
>> What particular interfaces are an issue here?
>
> All of them? For example, the policydb.c code just changed to use 
> DEBUG...
> policydb_read()
> policydb_write()
> everything else...


Well... I guess those would be in the static library. In fact, the 
shared library doesn't provide a whole lot of functionality right now:

  global: sepol_genbools*; sepol_set_policydb_from_file; 
sepol_check_context; sepol_genusers; sepol_debug; sepol_set_delusers;

So, if you deprecate the shared sepol_gen*, you're left 
with...sepol_check_context (which makes use of sepol_context_to_sid, and 
sepol_ctx_*)... and also sepol_set_policydb_from_file (which I think 
should also be deprecated, along with the global policydb).

Why are we making this a static library again?



--
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 ] Move more things to newer debug system

[Stephen Smalley]

On Thu, 2005-09-15 at 03:31 -0400, Ivan Gyurdiev wrote:
> Ivan Gyurdiev wrote:
> > All of them? For example, the policydb.c code just changed to use 
> > DEBUG...
> > policydb_read()
> > policydb_write()
> > everything else...
> 
> 
> Well... I guess those would be in the static library. In fact, the 
> shared library doesn't provide a whole lot of functionality right now:
> 
>   global: sepol_genbools*; sepol_set_policydb_from_file; 
> sepol_check_context; sepol_genusers; sepol_debug; sepol_set_delusers;

That's right.  Remember, libsepol is just a blob of code taken from
checkpolicy so that we could allow very limited re-use of its knowledge
of policy internals by specific programs that (unlike checkpolicy) were
not themselves tightly coupled to policy internals.  libsepol was
originally created to allow init and load_policy to patch the in-memory
policy for local boolean settings, then later leveraged to allow
setfiles to validate file_contexts against a binary policy during policy
builds, and then later leveraged to allow init and load_policy to patch
the in-memory policy for local user settings.

> So, if you deprecate the shared sepol_gen*, you're left 
> with...sepol_check_context (which makes use of sepol_context_to_sid, and 
> sepol_ctx_*)... and also sepol_set_policydb_from_file (which I think 
> should also be deprecated, along with the global policydb).
> 
> Why are we making this a static library again?

If we separate the kernel binary policy file from the rpm-managed one,
and push all policy changes (including local boolean and user settings
as well as module management) through the policy toolchain to rebuild
the kernel binary policy file, then init and load_policy no longer need
libsepol at all.  So the only user left of the shared libsepol is
setfiles, which might mean that we do keep a shared libsepol just for
it, but limited to those two interfaces (and a global policydb presents
no problem there nor should it for the userspace security server as we
don't need per-thread settings for it, so I see no value in changing
it).

On the other hand, libsemanage incorporates libsepol, currently
statically, and if libsemanage eventually provides a shared library,
then we will another "shared" instance of libsepol, just hidden behind
the libsemanage interface.  Tresys already moved the module read/write
code from libsemanage to libsepol, so possibly with just a little more
work, libsemanage could be made less tightly coupled to libsepol
internals and use a shared libsepol instead of incorporating libsepol
directly.

-- 
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 ] Move more things to newer debug system

[Stephen Smalley]

On Thu, 2005-09-15 at 03:31 -0400, Ivan Gyurdiev wrote:
> So, if you deprecate the shared sepol_gen*, you're left 
> with...sepol_check_context (which makes use of sepol_context_to_sid, and 
> sepol_ctx_*)... and also sepol_set_policydb_from_file (which I think 
> should also be deprecated, along with the global policydb).

BTW, keep in mind that most programs that want to check context validity
are going to use security_check_context() from libselinux, which has no
threading issues.  It is only specialized cases where you specifically
want to check against a binary policy rather than the active kernel
policy that you need the libsepol functions.

-- 
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 ] Move more things to newer debug system

[Stephen Smalley]

On Wed, 2005-09-14 at 15:37 -0400, Ivan Gyurdiev wrote:
> So, I'm not clear on the exact migration steps that need to be taken here.
> I've just changed a lot of printfs to DEBUG calls. I'm not entirely sure 
> whether
> they qualify as error reporting or debugging (probably the former). In 
> fact,
> the distinction is still a bit lost to me - it seems as if all current 
> uses of DEBUG
> are for error handling, and none are for debugging. Perhaps you could 
> give an
> example to clarify the situation...
> 
> Next, what to do we do with all those printf calls that remain, and the ones
> that are now DEBUG calls.... if they are in fact error-reporting statements,
> then I understand you want them written in an error buffer. Where does
> the buffer come from, given the current API?
> 
> I am in favor of moving all messaging systems into one .c and .h file,
> and making things more consistent.  I also like the idea of output
> levels for debugging and error-messaging, although I'm not entirely sure
> where I would use which.

Ok, suggestion:
- Leave write_error() and DEBUG() separate,
- Move write_error() to a unified interface and shared implementation,
and have it internally call DEBUG() as well as saving the message in the
buffer,
- Convert remaining printf/fprintf occurrences to DEBUG(),
- Convert direct snprintf calls into error buffers to use the shared
write_error() interface.

-- 
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 ] Move more things to newer debug system

[Ivan Gyurdiev]

>Ok, suggestion:
>- Leave write_error() and DEBUG() separate,
>- Move write_error() to a unified interface and shared implementation,
>and have it internally call DEBUG() as well as saving the message in the
>buffer,
>- Convert remaining printf/fprintf occurrences to DEBUG(),
>- Convert direct snprintf calls into error buffers to use the shared
>write_error() interface.
>  
>
Do you see DEBUG as being off by default or on by default?
I'm trying to understand what the role of DEBUG is here... currently it 
handles
important printfs that will disappear if turned off.

Also, should write_error be paying attention to security_debug() ?

--
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 ] Move more things to newer debug system

[Stephen Smalley]

On Thu, 2005-09-15 at 12:03 -0400, Ivan Gyurdiev wrote:
> >Ok, suggestion:
> >- Leave write_error() and DEBUG() separate,
> >- Move write_error() to a unified interface and shared implementation,
> >and have it internally call DEBUG() as well as saving the message in the
> >buffer,
> >- Convert remaining printf/fprintf occurrences to DEBUG(),
> >- Convert direct snprintf calls into error buffers to use the shared
> >write_error() interface.
> >  
> >
> Do you see DEBUG as being off by default or on by default?
> I'm trying to understand what the role of DEBUG is here... currently it 
> handles
> important printfs that will disappear if turned off.

I think on by default, but still with the ability for explicit disable,
which will likely be used by the policy server.

> Also, should write_error be paying attention to security_debug() ?

No, I think write_error should always write the formatted message to the
error buffer, regardless of debug settings.  Then it calls DEBUG(),
which does its thing.  Hence, all error messages find their way to DEBUG
eventually (which may then discard them based on its setting), whereas
write_error is called more selectively and only when the calling
interface has error buffer arguments available.

-- 
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 ] Move more things to newer debug system

[Luke Kenneth Casson Leighton]

On Wed, Sep 14, 2005 at 03:37:00PM -0400, Ivan Gyurdiev wrote:
> 
> >So, do you mean taking write_error, optionally passing in a string and 
> >size, having it write to the string if it's present and then calling 
> >the debug function pointer if it is present? That way we'll get the 
> >error in the handle so that the caller can deal with it and the debug 
> >function would also be called. Then the caller could choose to use 
> >debug by setting the pointer and other apps that need an error in the 
> >handle could use that?
> >
> >This seems reasonable to me.. Ivan?

 can i make a suggestion, namely that an old-looking api be written
 in terms of the "new" api?

 l.


--
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 ] Move more things to newer debug system

[Luke Kenneth Casson Leighton]

On Wed, Sep 14, 2005 at 01:16:55PM -0400, Ivan Gyurdiev wrote:
> 
> >BTW, I just noticed - aren't the calls to sepol_debug() by
> >sepol_enable/disable_debug() broken?  If you just set the function
> >pointer, you don't want to clobber it using the old interface.
> >
> > 
> >
> The old interface is supposed to go away, but until it does, the new 
> interface must provide the same functionality, which is why it maintains 
> printing to stdout using the old system. 

 define old interface in terms of new one - including providing a debug
 function that writes to stdout.

 not only will this maintain a legacy api but also it will give
 people some example code to cut/paste into their updated app
 that uses the new api.

 l.

--
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 ] Move more things to newer debug system

[Luke Kenneth Casson Leighton]

On Wed, Sep 14, 2005 at 11:38:45AM -0400, Stephen Smalley wrote:
> On Wed, 2005-09-14 at 11:33 -0400, Joshua Brindle wrote:
> > Omitting it isn't my concern if it is really for debugging, I do not 
> > think that we should try to use the debug system for error reporting to 
> > the originating function if it is going to be a debug system.
> 
> I was thinking that unifying them would be ideal, and the callbacks can
> decide how they want to filter the results.  Just requires that the
> callbacks be provided with a level indicator in addition to the other
> arguments that indicates whether it is debug info only or error info.
> Not unlike syslog(3) or printk(9).
 
 okay.

 exactly how much complexity are we getting into, here, because if it's
 getting ... "large" then i would strongly recommend looking at the
 debugging infrastructure utilised in FreeDCE (actually DCE 1.1) and
 samba:

 #define RPC_DBG_FUNCTIONALITY_AREA_1 1
 #define RPC_DBG_FUNCTIONALITY_AREA_2 2
 #define RPC_DBG_MEMORY_DEBUGGING_FOR_LIBSEPOL 3
 #define RPC_DBG_WEIRD_STUFF_A 4
 #define RPC_DBG_MAX 4

 #define CRITICALL_LEVEL 0
 #define INFORMATIONAL_LEVEL 10

 RPC_DBG_PRINTF(RPC_DBG_MEMORY_DEBUGGING_FOR_LIBSEPOL,
			 INFORMATIONAL_LEVEL,
              ("theactualdebugprintfstuff %s\n", error_msg));


 then you have an array of debug levels:

 static rpc_g_dbg_levels[RPC_DBG_MAX]

 and when you start an application, you start it with an argument
 as follows:

 --debug "0-1=0,3=10"

 then the RPC_DBG_PRINTF uses its two arguments to check the array of
 debug levels to find out if you should print out the message.


 the reason why DCE/RPC and samba utilise this type of debugging system
 is... well... FreeDCE has about SIXTY separate #defines for different
 debugging purposes.

 if all of that information was splattered across your console, it'd be
 utterly useless.

 so the above cuts out the crap you don't want.

 l.


--
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 ] Move more things to newer debug system

[Ivan Gyurdiev]

>No, I think write_error should always write the formatted message to the
>error buffer, regardless of debug settings.  Then it calls DEBUG(),
>which does its thing.  Hence, all error messages find their way to DEBUG
>eventually (which may then discard them based on its setting), whereas
>write_error is called more selectively and only when the calling
>interface has error buffer arguments available.
>  
>
That sounds like a good plan to convert DEBUG into a debugging interface,
and write_error into an error reporting interface that relays the errors 
into the DEBUG channel too.

However, the messages currently being sent to DEBUG should be considered 
error reporting IMHO...
"couldn't open file X.Y.Z"
"boolean N is no longer valid"
"policy is the wrong 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: [ SEPOL ] Move more things to newer debug system

[Ivan Gyurdiev]

>I was thinking that unifying them would be ideal, and the callbacks can
>decide how they want to filter the results.  Just requires that the
>callbacks be provided with a level indicator in addition to the other
>arguments that indicates whether it is debug info only or error info.
>Not unlike syslog(3) or printk(9).
>  
>
Which messages would be marked at what levels?
Example?


--
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 ] Move more things to newer debug system

[Ivan Gyurdiev]

Ivan Gyurdiev wrote:

>
>> I was thinking that unifying them would be ideal, and the callbacks can
>> decide how they want to filter the results.  Just requires that the
>> callbacks be provided with a level indicator in addition to the other
>> arguments that indicates whether it is debug info only or error info.
>> Not unlike syslog(3) or printk(9).
>>  
>>
> Which messages would be marked at what levels?
> Example?


Never mind, you already answered that question.

> Offhand, I don't know of an actual debugging user of DEBUG() yet,
> although it seems that the if (state->verbose) printf statements in the
> Tresys code would qualify as potential debugging users.  There are also
> "informational" cases, like the printfs in policydb_index_others.
> syslog(3) does have distinct notions of debug, info, warning, error,
> critical, alert, and emergency as its priority values.

I'm not sure we need both a filtering mechanism, and two systems - one 
for debugging and one for error reporting.  That looks like redundancy 
to me. I think we need to choose one debug system, add a filtering 
mechanism to it, and convert everything to use it. The filtering 
mechanism would then determine what's done with the different types of 
messages (which are not too clear at the moment).

There's the issue of global vs per-function-state. I think the second 
one wins here, and we'll just have to adjust the shared APIs. We could 
create something similar to the semanage handle, which encompasses a 
policydb, as well as error handling. I think libsepol should be the 
library that manages the policydb object.

There's the issue of multiple errors - I think any error system needs to 
support multiple error messages.

Then there's the issue of buffer vs callback. I think callback wins 
here. Provided function-local state, there's no reason not to have a 
callback. If you want buffering services, you can arrange for that in 
the callback. We should add a void* argument to the callback, and keep 
that in the state object, passing it at every invocation.

The function name.... is irrelevant if we go with the callback system - 
callback can drop the name. I think it's useful, and it should stay.

Callback should look something like this, maybe ?
We can write macros to reduce arguments...

int (*DEBUG_v2) (
    void* arg,               /* Caller supplied argument */
    int level,                 /* WARN, ERROR, DEBUG ...kind of like MLS 
level - notion of rank */
    int channel,            /* Wine has this...not sure we need it... 
similar to MLS category - area of debugging */
    const char* fname,  /* Function name */
    const char* fmt,      /* Format string */
    ...)                         /* Format arguments */
   
   










--
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 ] Move more things to newer debug system

[Ivan Gyurdiev]

> Callback should look something like this, maybe ?
> We can write macros to reduce arguments...

For example, we can replace the level argument with a macro,
and have the function name be the level - WARN, FIXME, DEBUG...etc.

>
> int (*DEBUG_v2) (
>    void* arg,               /* Caller supplied argument */
>    int level,                 /* WARN, ERROR, DEBUG ...kind of like 
> MLS level - notion of rank */
>    int channel,            /* Wine has this...not sure we need it... 
> similar to MLS category - area of debugging */
>    const char* fname,  /* Function name */
>    const char* fmt,      /* Format string */
>    ...)                         /* Format arguments */
>    



--
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 ] Move more things to newer debug system

[Stephen Smalley]

On Sat, 2005-09-17 at 23:52 -0400, Ivan Gyurdiev wrote:
> I'm not sure we need both a filtering mechanism, and two systems - one 
> for debugging and one for error reporting.  That looks like redundancy 
> to me. I think we need to choose one debug system, add a filtering 
> mechanism to it, and convert everything to use it. The filtering 
> mechanism would then determine what's done with the different types of 
> messages (which are not too clear at the moment).

I viewed it more as:
- drop the notion of level/priority since you explained how you can't
truly classify the different DEBUG messages into those categorizations,
- don't clutter a single interface trying to support both variants, just
keep them as separate interfaces,
- recognize that both DEBUG and write_error are doing error reporting,
but with differing feedback mechanisms (global callback vs. per-call
error buffering) and with differing levels of detail.

I expected to use DEBUG() pervasively as the default mechanism, and only
selectively insert write_error calls (and extend interfaces to take the
buffers) as needed to support the specific needs of the policy
management daemon.  Policy management daemon would then disable the
DEBUG output or redirect it to a log file and only feed the buffered
error messages back to the client.

But if you really want to go down this road...

> There's the issue of global vs per-function-state. I think the second 
> one wins here, and we'll just have to adjust the shared APIs. We could 
> create something similar to the semanage handle, which encompasses a 
> policydb, as well as error handling. I think libsepol should be the 
> library that manages the policydb object.

The second one is more general, although I'm not entirely convinced it
is needed.  I agree that having libsepol properly encapsulate the
policydb would be good.

> There's the issue of multiple errors - I think any error system needs to 
> support multiple error messages.

Agreed.

> Then there's the issue of buffer vs callback. I think callback wins 
> here. Provided function-local state, there's no reason not to have a 
> callback. If you want buffering services, you can arrange for that in 
> the callback. We should add a void* argument to the callback, and keep 
> that in the state object, passing it at every invocation.

I agree with this as well; the current error buffers are clearly
unsatisfactory for assertion and hierarchy checking.

> The function name.... is irrelevant if we go with the callback system - 
> callback can drop the name. I think it's useful, and it should stay.

Yes.

> Callback should look something like this, maybe ?
> We can write macros to reduce arguments...
> 
> int (*DEBUG_v2) (
>     void* arg,               /* Caller supplied argument */
>     int level,                 /* WARN, ERROR, DEBUG ...kind of like MLS 
> level - notion of rank */
>     int channel,            /* Wine has this...not sure we need it... 
> similar to MLS category - area of debugging */
>     const char* fname,  /* Function name */
>     const char* fmt,      /* Format string */
>     ...)                         /* Format arguments */

If we use parallels to the existing syslog(3) levels (priorities), then
the callback can easily map the level/priority values and forward them
along to syslog(3) 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: [ SEPOL ] Move more things to newer debug system

[Ivan Gyurdiev]

Stephen Smalley wrote:

>On Sat, 2005-09-17 at 23:52 -0400, Ivan Gyurdiev wrote:
>  
>
>>I'm not sure we need both a filtering mechanism, and two systems - one 
>>for debugging and one for error reporting.  That looks like redundancy 
>>to me. I think we need to choose one debug system, add a filtering 
>>mechanism to it, and convert everything to use it. The filtering 
>>mechanism would then determine what's done with the different types of 
>>messages (which are not too clear at the moment).
>>    
>>
>
>I viewed it more as:
>- drop the notion of level/priority since you explained how you can't
>truly classify the different DEBUG messages into those categorizations,
>  
>
I was saying that I'm not convinced you can consider the lowest-level 
message of type ERROR, and the ones above it in the call chain of type 
DEBUG. A different classification system might work, however... the 
level argument could be useful.

>- don't clutter a single interface trying to support both variants, just
>keep them as separate interfaces,
>  
>
We could easily move the level argument into the function name (support 
WARN(), ERROR()), etc.

>- recognize that both DEBUG and write_error are doing error reporting,
>but with differing feedback mechanisms (global callback vs. per-call
>error buffering) and with differing levels of detail.
>  
>
This is the important part - do we want to keep both feedback 
mechanisms, or not.
If the former, then yes - we should keep both write_error and DEBUG. 
However, I think
that's not necessary, because if we implement callbacks, and add a state 
object to pass to
the callbacks, the user of libsepol/libsemanage can design any error 
system they feel like,
which is more flexible than making that choice in the library.

Regarding the differing levels of detail - the callback can make that 
decision if we provide it
with the level of  message (error/debug..etc).

>I expected to use DEBUG() pervasively as the default mechanism, and only
>selectively insert write_error calls (and extend interfaces to take the
>buffers) as needed to support the specific needs of the policy
>management daemon.  Policy management daemon would then disable the
>DEBUG output or redirect it to a log file and only feed the buffered
>error messages back to the client.
>  
>
If we design the debug system properly, we can handle any usage pattern, 
including this one.


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

Re: [ SEPOL ] Move more things to newer debug system

[Stephen Smalley]

On Mon, 2005-09-19 at 10:05 -0400, Ivan Gyurdiev wrote:
> I was saying that I'm not convinced you can consider the lowest-level 
> message of type ERROR, and the ones above it in the call chain of type 
> DEBUG. A different classification system might work, however... the 
> level argument could be useful.

True.

> We could easily move the level argument into the function name (support 
> WARN(), ERROR()), etc.

Yes.  The larger issue is introducing the state object arguments to the
interfaces and propagating them down to all DEBUG() calls.

> However, I think
> that's not necessary, because if we implement callbacks, and add a state 
> object to pass to
> the callbacks, the user of libsepol/libsemanage can design any error 
> system they feel like,
> which is more flexible than making that choice in the library.
> 
> Regarding the differing levels of detail - the callback can make that 
> decision if we provide it
> with the level of  message (error/debug..etc).

Yes, if we can usefully make such distinctions (due to the need to
provide higher level context to understand the lowest level errors).
What do you expect from the callbacks?  They can certainly do simple
things like drop the entire message based on level, drop the function
name information, etc, but constructing an overall error message that is
presentable to the client/user from the entire set of DEBUG() calls
seems difficult.
> 
> >I expected to use DEBUG() pervasively as the default mechanism, and only
> >selectively insert write_error calls (and extend interfaces to take the
> >buffers) as needed to support the specific needs of the policy
> >management daemon.  Policy management daemon would then disable the
> >DEBUG output or redirect it to a log file and only feed the buffered
> >error messages back to the client.
> >  
> >
> If we design the debug system properly, we can handle any usage pattern, 
> including this one.

True, but if this is the only usage pattern (in addition to the default
one of writing all DEBUG output to the user and not using the error
buffers at all for single-threaded users), then it seems overkill.

-- 
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 ] Move more things to newer debug system

[Ivan Gyurdiev]

>What do you expect from the callbacks? 
>
That's a good question - I'm not entirely sure. I guess that's why I'm 
trying to push the issue outside the library so we don't have to make 
that kind of decision.

It's important, IMHO, that the user not be forced to use stdout/stderr 
for debugging. I think both approaches accomplish that. Then it's 
important to allow the caller to recognize which thread created the 
problem - that's Joshua's concern. This means that we *need* 
caller-supplied state at the point of debugging, or we can't accomplish 
this (whether that's the address of a per-thread buffer, or whether it's 
the thread ID available to a callback function, or whatever).

The issue of callback vs buffer is less important - I guess it seems 
more flexible to me to have a callback function, instead of having to 
worry about allocating buffers, looking inside the buffers, deallocating 
buffers... but if you disagree we can use buffers.

>They can certainly do simple
>things like drop the entire message based on level, drop the function
>name information, etc, but constructing an overall error message that is
>presentable to the client/user from the entire set of DEBUG() calls
>seems difficult.
>  
>
Yes, I wouldn't expect the callback to have the logic to do that - 
that's just a question of policy on how you use the debug system in your 
code, and at which point you issue error messages.

>>If we design the debug system properly, we can handle any usage pattern, 
>>including this one.
>>    
>>
>
>True, but if this is the only usage pattern (in addition to the default
>one of writing all DEBUG output to the user and not using the error
>buffers at all for single-threaded users), then it seems overkill.
>  
>
Which part, specifically, seems like overkill?




--
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 ] Move more things to newer debug system

[Stephen Smalley]

On Mon, 2005-09-19 at 12:24 -0400, Ivan Gyurdiev wrote:
> That's a good question - I'm not entirely sure. I guess that's why I'm 
> trying to push the issue outside the library so we don't have to make 
> that kind of decision.
> 
> It's important, IMHO, that the user not be forced to use stdout/stderr 
> for debugging. I think both approaches accomplish that. Then it's 
> important to allow the caller to recognize which thread created the 
> problem - that's Joshua's concern. This means that we *need* 
> caller-supplied state at the point of debugging, or we can't accomplish 
> this (whether that's the address of a per-thread buffer, or whether it's 
> the thread ID available to a callback function, or whatever).
> 
> The issue of callback vs buffer is less important - I guess it seems 
> more flexible to me to have a callback function, instead of having to 
> worry about allocating buffers, looking inside the buffers, deallocating 
> buffers... but if you disagree we can use buffers.

No, I agree with using callbacks.

> Yes, I wouldn't expect the callback to have the logic to do that - 
> that's just a question of policy on how you use the debug system in your 
> code, and at which point you issue error messages.

Yes, but if the messages are generated in such a form that you need them
all (or at least pieces of several of them) to make sense of the overall
error, then it becomes difficult to implement a sensible callback that
does anything other than output/log/buffer them all (optionally without
the function information).  They can certainly differentiate between
LOG_INFO, LOG_DEBUG, and LOG_ERR, but I think all current DEBUG calls
would be LOG_ERR.

> Which part, specifically, seems like overkill?

If the policy management daemon was going to just discard all messages
except for the leaf node error handling ones, then adding a state object
to the interfaces and pushing it down to all DEBUG calls seemed
pointless; the result would never be used for most of them.  OTOH, you
would still need the state object for all interfaces used by the policy
management daemon, and you would need to push it down to all leaf node
error handlers (whether they would call DEBUG or a separate write_error
interface), so I suppose it might not make a large difference.

-- 
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 ] Move more things to newer debug system

[Ivan Gyurdiev]

>>Yes, I wouldn't expect the callback to have the logic to do that - 
>>that's just a question of policy on how you use the debug system in your 
>>code, and at which point you issue error messages.
>>    
>>
>
>Yes, but if the messages are generated in such a form that you need them
>all (or at least pieces of several of them) to make sense of the overall
>error, then it becomes difficult to implement a sensible callback that
>does anything other than output/log/buffer them all (optionally without
>the function information).  They can certainly differentiate between
>LOG_INFO, LOG_DEBUG, and LOG_ERR, but I think all current DEBUG calls
>would be LOG_ERR.
>  
>
Well, perhaps I should make an effort to generate errors in a different 
pattern, so the end result is more user friendly (possibly pass down 
more information to the leaf, so it can generate a single error report). 
In any case, I am concerned about semanage code here, specifically, 
semanage code that hasn't been merged yet that uses polymorphism. I'm 
not sure how much of a problem the current sepol code is...

>>Which part, specifically, seems like overkill?
>>    
>>
>
>If the policy management daemon was going to just discard all messages
>except for the leaf node error handling ones, then add ng a state object
>to the interfaces and pushing it down to all DEBUG calls seemed
>pointless; the result would never be used for most of them.  OTOH, you
>would still need the state object for all interfaces used by the policy
>management daemon, and you would need to push it down to all leaf node
>error handlers (whether they would call DEBUG or a separate write_error
>interface), so I suppose it might not make a large difference.
>  
>
Yes, you would still need the state object at the leaf node. 
Alternatively you could make use of more informative error codes in the 
leaf node, and print errors higher up. You need the state object at the 
point where the error is printed.

I think if we should wrap policydb into this so-called "state object" 
for the added benefit of eliminating the global policydb.

--
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 ] Move more things to newer debug system

[Stephen Smalley]

On Mon, 2005-09-19 at 13:16 -0400, Ivan Gyurdiev wrote:
> Well, perhaps I should make an effort to generate errors in a different 
> pattern, so the end result is more user friendly (possibly pass down 
> more information to the leaf, so it can generate a single error report).

I think your later idea is better (passing up just error code from the
low level primitives, constructing the single error report in the higher
level).
 
> Yes, you would still need the state object at the leaf node. 
> Alternatively you could make use of more informative error codes in the 
> leaf node, and print errors higher up. You need the state object at the 
> point where the error is printed.

Yes, this approach seems preferable.  Then the call chain info can
become just debug-level info, and omitted from error reports to users /
clients.

> I think if we should wrap policydb into this so-called "state object" 
> for the added benefit of eliminating the global policydb.

As long as you can separately create the policydb, populate it, and
associate it with these state objects, so that the program can still use
a single shared policydb if desired, this is fine.  Userspace security
server shouldn't have a per-thread policydb any more than the kernel
security server does.

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