[ SEPOL/SEMANAGE ] Boolean record

[ SEPOL/SEMANAGE ] Boolean record

[Ivan Gyurdiev]

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

The attached patches replace the boolinfo structure with a record.
They also fix improper use of DEBUG, which would cause corrupt error 
messages.


[-- Attachment #2: libsemanage.boolean.record.diff --]
[-- Type: text/x-patch, Size: 1998 bytes --]

diff -Naur libsemanage/include/semanage/boolean_record.h libsemanage.new/include/semanage/boolean_record.h
--- libsemanage/include/semanage/boolean_record.h	1969-12-31 19:00:00.000000000 -0500
+++ libsemanage.new/include/semanage/boolean_record.h	2005-09-20 03:29:38.000000000 -0400
@@ -0,0 +1,67 @@
+#ifndef _SEMANAGE_BOOLEAN_RECORD_H_
+#define _SEMANAGE_BOOLEAN_RECORD_H_
+
+/* Implementation via sepol */
+#include <sepol/bool_record.h>
+typedef sepol_bool_t semanage_bool_t;
+typedef sepol_bool_key_t semanage_bool_key_t;
+
+/* Key */
+static inline
+int semanage_bool_key_create(const char* name, semanage_bool_key_t* key) {
+	return sepol_bool_key_create(name, key);
+}
+
+static inline
+int semanage_bool_key_extract(semanage_bool_t boolean, semanage_bool_key_t* key) {
+	return sepol_bool_key_extract(boolean, key);
+}
+
+static inline 
+void semanage_bool_key_free(semanage_bool_key_t key) {
+	sepol_bool_key_free(key);
+}
+
+static inline
+int semanage_bool_compare(semanage_bool_t boolean, semanage_bool_key_t key) {
+	return sepol_bool_compare(boolean, key);
+}
+	
+/* Name */
+static inline
+const char* semanage_bool_get_name(semanage_bool_t boolean) {
+	return sepol_bool_get_name(boolean);
+}
+
+static inline
+int semanage_bool_set_name(semanage_bool_t boolean, const char* name) {
+	return sepol_bool_set_name(boolean, name);
+}
+
+/* Value */
+static inline
+int semanage_bool_get_value(semanage_bool_t boolean) {
+	return sepol_bool_get_value(boolean);
+}
+
+static inline
+void semanage_bool_set_value(semanage_bool_t boolean, int value) {
+	sepol_bool_set_value(boolean, value);
+}
+
+/* Create/Clone/Destroy */
+static inline
+int semanage_bool_create(semanage_bool_t* bool_ptr) {
+	return sepol_bool_create(bool_ptr);
+}
+
+static inline
+int semanage_bool_clone(semanage_bool_t boolean, semanage_bool_t* bool_ptr) {
+	return sepol_bool_clone(boolean, bool_ptr);
+}
+
+static inline
+void semanage_bool_free(semanage_bool_t boolean) {
+	sepol_bool_free(boolean);
+}
+#endif

[-- Attachment #3: libsepol.boolean.record.diff --]
[-- Type: text/x-patch, Size: 7841 bytes --]

diff -Naur libsepol/include/sepol/boolean_record.h libsepol.new/include/sepol/boolean_record.h
--- libsepol/include/sepol/boolean_record.h	1969-12-31 19:00:00.000000000 -0500
+++ libsepol.new/include/sepol/boolean_record.h	2005-09-20 03:20:51.000000000 -0400
@@ -0,0 +1,40 @@
+#ifndef _SEPOL_BOOLEAN_RECORD_H_
+#define _SEPOL_BOOLEAN_RECORD_H_
+
+#include <stddef.h>
+
+struct sepol_bool;
+struct sepol_bool_key;
+typedef struct sepol_bool* sepol_bool_t;
+typedef struct sepol_bool_key* sepol_bool_key_t;
+
+/* Key */
+extern int sepol_bool_key_create(
+	const char* name,
+	sepol_bool_key_t* key);
+
+extern int sepol_bool_key_extract(
+	sepol_bool_t boolean,
+	sepol_bool_key_t* key_ptr);
+
+extern void sepol_bool_key_free(
+	sepol_bool_key_t key);
+
+extern int sepol_bool_compare(
+	sepol_bool_t boolean,
+	sepol_bool_key_t key);
+	
+/* Name */
+extern const char* sepol_bool_get_name(sepol_bool_t boolean);
+extern int sepol_bool_set_name(sepol_bool_t boolean, const char* name);
+
+/* Value */
+extern int sepol_bool_get_value(sepol_bool_t boolean);
+extern void sepol_bool_set_value(sepol_bool_t boolean, int value);
+
+/* Create/Clone/Destroy */
+extern int sepol_bool_create(sepol_bool_t* bool_ptr);
+extern int sepol_bool_clone(sepol_bool_t boolean, sepol_bool_t* bool_ptr);
+extern void sepol_bool_free(sepol_bool_t boolean);
+
+#endif
diff -Naur libsepol/include/sepol/booleans.h libsepol.new/include/sepol/booleans.h
--- libsepol/include/sepol/booleans.h	2005-07-26 14:43:29.000000000 -0400
+++ libsepol.new/include/sepol/booleans.h	2005-09-20 03:20:49.000000000 -0400
@@ -2,22 +2,17 @@
 #define _SEPOL_BOOLEANS_H_
 
 #include <sepol/policydb.h>
-
-/* High level representation of a boolean */
-typedef struct sepol_boolinfo {
-	char* name;
-	int value;
-} sepol_boolinfo_t;
+#include <sepol/boolean_record.h>
 
 /* Load a boolean into the policy */
 extern int sepol_bool_load (
 	policydb_t* policydb, 
-	sepol_boolinfo_t* boolean);
+	sepol_bool_t boolean);
 
 /* Load a boolean array into the policy */
 extern int sepol_bool_load_array(
 	policydb_t* policydb,
-	sepol_boolinfo_t* bool_arr,
+	sepol_bool_t* bool_arr,
 	int bool_arr_len);
 
-#endif /* _SEPOL_BOOLEANS_H_ */
+#endif
diff -Naur libsepol/src/boolean_record.c libsepol.new/src/boolean_record.c
--- libsepol/src/boolean_record.c	1969-12-31 19:00:00.000000000 -0500
+++ libsepol.new/src/boolean_record.c	2005-09-20 03:12:40.000000000 -0400
@@ -0,0 +1,133 @@
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <sepol/boolean_record.h>
+#include "debug.h"
+
+struct sepol_bool {
+	/* This boolean's name */
+	char* name;
+
+	/* Its value */
+	int value;
+};
+
+struct sepol_bool_key {
+	/* This boolean's name */
+	const char* name;
+};
+
+int sepol_bool_key_create(
+	const char* name,
+	sepol_bool_key_t* key_ptr) {
+
+	sepol_bool_key_t tmp_key = 
+		(sepol_bool_key_t) malloc(sizeof (struct sepol_bool_key));
+
+	if (!tmp_key) {
+		DEBUG(__FUNCTION__, "out of memory, "
+			"could not create boolean key\n");
+		return STATUS_ERR;
+	}
+
+	tmp_key->name = name;
+
+	*key_ptr = tmp_key;
+	return STATUS_SUCCESS;
+}
+
+int sepol_bool_key_extract(sepol_bool_t boolean, sepol_bool_key_t* key_ptr) {
+	if (sepol_bool_key_create(boolean->name, key_ptr) < 0) {
+		DEBUG(__FUNCTION__, "could not extract key from boolean %s\n",
+			boolean->name);
+		return STATUS_ERR;
+	}
+
+	return STATUS_SUCCESS;
+}	
+
+void sepol_bool_key_free(sepol_bool_key_t key) {
+	free(key);
+}
+
+int sepol_bool_compare(
+	sepol_bool_t boolean,
+	sepol_bool_key_t key) {
+	
+	if (!strcmp(boolean->name, key->name))
+		return 0;
+	return 1;
+}
+
+/* Name */
+const char* sepol_bool_get_name(sepol_bool_t boolean) {
+	return boolean->name;
+}
+
+int sepol_bool_set_name(sepol_bool_t boolean, const char* name) {
+	boolean->name = strdup(name);
+	if (!boolean->name) {
+		DEBUG(__FUNCTION__, "out of memory, "
+			"could not set boolean name\n");
+		return STATUS_ERR;
+	}
+	return STATUS_SUCCESS;
+}
+
+/* Value */
+int sepol_bool_get_value(sepol_bool_t boolean) {
+	return boolean->value;
+}
+
+void sepol_bool_set_value(sepol_bool_t boolean, int value) {
+	boolean->value = value;
+}
+
+/* Create */
+int sepol_bool_create(sepol_bool_t* bool_ptr) {
+	sepol_bool_t boolean = (sepol_bool_t) 
+		malloc(sizeof (struct sepol_bool));
+
+        if (!boolean) {
+		DEBUG(__FUNCTION__, "out of memory, "
+			"could not create boolean record\n"); 
+		return STATUS_ERR;
+	}
+
+        boolean->name = NULL;
+	boolean->value = 0;
+	
+	*bool_ptr = boolean;
+	return STATUS_SUCCESS;
+}
+
+/* Deep copy clone */
+int sepol_bool_clone(sepol_bool_t boolean, sepol_bool_t* bool_ptr) {
+	sepol_bool_t new_bool = NULL;
+
+	if (sepol_bool_create(&new_bool) < 0)
+		goto err;
+
+	if (sepol_bool_set_name(new_bool, boolean->name) < 0)
+		goto err;
+
+	new_bool->value = boolean->value;
+
+	*bool_ptr = new_bool;
+	return STATUS_SUCCESS;
+
+	err:
+	DEBUG(__FUNCTION__, "could not clone boolean record\n");
+	sepol_bool_free(new_bool);
+	return STATUS_ERR;
+}
+
+/* Destroy */
+void sepol_bool_free(sepol_bool_t boolean) {
+	if (!boolean)
+		return;
+	
+	free(boolean->name);
+	free(boolean);
+}
diff -Naur libsepol/src/booleans.c libsepol.new/src/booleans.c
--- libsepol/src/booleans.c	2005-07-26 14:43:29.000000000 -0400
+++ libsepol.new/src/booleans.c	2005-09-20 03:26:44.000000000 -0400
@@ -9,29 +9,42 @@
 #include <sepol/hashtab.h>
 #include <sepol/policydb.h>
 #include <sepol/conditional.h>
+#include <sepol/boolean_record.h>
 
 static inline int bool_update (
 	policydb_t* policydb,
-	sepol_boolinfo_t* boolean) {
+	sepol_bool_t boolean) {
+
+	char* name = strdup(sepol_bool_get_name(boolean));
+	int value = sepol_bool_get_value(boolean);
+
+	if (!name) {
+		DEBUG(__FUNCTION__, "out of memory\n");
+		goto err;
+	}
 
 	cond_bool_datum_t *datum = 
-		hashtab_search(policydb->p_bools.table, boolean->name);
+		hashtab_search(policydb->p_bools.table, name);
 	if (!datum) {
-		DEBUG(__FUNCTION__, "boolean %s no longer in policy\n",
-			boolean->name);
-		return STATUS_ERR;
-        }
-	if (boolean->value != 0 && boolean->value != 1) {
-		DEBUG(__FUNCTION__, "illegal value %d for boolean %s\n",
-			boolean->value, boolean->name);
-		return STATUS_ERR;
-        }
-        datum->state = boolean->value;
+		DEBUG(__FUNCTION__, "boolean %s no longer in policy\n", name);
+		goto err;
+	}
+	if (value != 0 && value != 1) {
+		DEBUG(__FUNCTION__, "illegal value %d for boolean %s\n", value, name);
+		goto err;
+	}
+	datum->state = value;
 	return STATUS_SUCCESS;
+
+	err:
+	free(name);
+	DEBUG(__FUNCTION__, "unable to update boolean %s\n", 
+		sepol_bool_get_name(boolean));
+	return STATUS_ERR;		
 }
 
 int sepol_bool_load (
-	policydb_t* policydb, sepol_boolinfo_t* boolean) {
+	policydb_t* policydb, sepol_bool_t boolean) {
 
 	if (bool_update(policydb, boolean) < 0)
 		goto err;	
@@ -44,27 +57,27 @@
 	return STATUS_SUCCESS;
 
 	err:
-	DEBUG(__FUNCTION__, "could not load boolean %s\n", boolean->name);
+	DEBUG(__FUNCTION__, "could not load boolean %s\n", 
+		sepol_bool_get_name(boolean));
 	errno = EINVAL;
 	return STATUS_ERR;
 }
 
 int sepol_bool_load_array(
 	policydb_t* policydb,
-	sepol_boolinfo_t* bool_arr,
+	sepol_bool_t* bool_arr,
 	int bool_arr_len) {	
 
 	int i, errors = 0;
 
 	for (i = 0; i < bool_arr_len; i++)
-		if (bool_update(policydb, &bool_arr[i]) < 0) {
+		if (bool_update(policydb, bool_arr[i]) < 0) {
 			errors++;
 			continue;
 		}	
 
 	if (evaluate_conds(policydb) < 0) {
-		DEBUG("%s: error while re-evaluating conditionals\n", 
-			__FUNCTION__);
+		DEBUG(__FUNCTION__, "error while re-evaluating conditionals\n");
 		goto err;
 	}
 
@@ -74,6 +87,6 @@
 	return STATUS_SUCCESS;
 	err:
 	errno = EINVAL;
-	DEBUG("%s: error while loading booleans\n", __FUNCTION__);
+	DEBUG(__FUNCTION__, "error while loading booleans\n");
 	return STATUS_ERR;
 }

Re: [ SEPOL/SEMANAGE ] Boolean record

[Stephen Smalley]

On Tue, 2005-09-20 at 03:40 -0400, Ivan Gyurdiev wrote:
> The attached patches replace the boolinfo structure with a record.
> They also fix improper use of DEBUG, which would cause corrupt error 
> messages.

Hi,

How useful is it to wrap these libsepol interfaces with libsemanage?

-- 
Stephen Smalley
National Security Agency


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

Re: [ SEPOL/SEMANAGE ] Boolean record

[Ivan Gyurdiev]

>How useful is it to wrap these libsepol interfaces with libsemanage?
>  
>
What do you mean?

The alternative is to either:
- implement the same interfaces in both places (which is worse), or
- have semanage functions take sepol data stuctures as arguments (which 
is bad design practice).

I don't think the current scheme is too bad - esp. because the wrap 
functions are inlined.

--
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/SEMANAGE ] Boolean record

[Stephen Smalley]

On Tue, 2005-09-20 at 15:35 -0400, Ivan Gyurdiev wrote:
> >How useful is it to wrap these libsepol interfaces with libsemanage?
> >  
> >
> What do you mean?
> 
> The alternative is to either:
> - implement the same interfaces in both places (which is worse), or
> - have semanage functions take sepol data stuctures as arguments (which 
> is bad design practice).

- keep the shared libsepol, and have applications use its interfaces
directly.

> I don't think the current scheme is too bad - esp. because the wrap 
> functions are inlined.

That's what makes it seem so pointless.  If the wrap functions weren't
inlined and actually added some semantics around the libsepol
implementation, then they might make sense.

-- 
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/SEMANAGE ] Boolean record

[Ivan Gyurdiev]

>- keep the shared libsepol, and have applications use its interfaces
>directly.
>
To clarify, are you saying that we should have?

semanage_user_add(semanage_handle_t handle, sepol_user_key_t key, 
sepol_user_t data)
semanage_user_del(semanage_handle_t handle, sepol_user_key_t key)
semanage_user_modify(semanage_handle_t handle, sepol_user_t key, 
sepol_user_t data)

The sepol and semanage functionality is different - it's just the data 
structure that's shared.

>That's what makes it seem so pointless.  If the wrap functions weren't
>inlined and actually added some semantics around the libsepol
>implementation, then they might make sense
>  
>
I don't know - it's just a design thing to allow future change - Joshua?
I'm not sure if the wrap functions add value, but at least they don't 
cause any harm.


--
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/SEMANAGE ] Boolean record

[Stephen Smalley]

On Tue, 2005-09-20 at 16:16 -0400, Ivan Gyurdiev wrote:
> >- keep the shared libsepol, and have applications use its interfaces
> >directly.
> >
> To clarify, are you saying that we should have?
> 
> semanage_user_add(semanage_handle_t handle, sepol_user_key_t key, 
> sepol_user_t data)
> semanage_user_del(semanage_handle_t handle, sepol_user_key_t key)
> semanage_user_modify(semanage_handle_t handle, sepol_user_t key, 
> sepol_user_t data)
> 
> The sepol and semanage functionality is different - it's just the data 
> structure that's shared.

If the data structures and interfaces are truly identical, then I see no
value in relaying them in this manner from libsepol to libsemanage.  I
understand wrapping them if we are going to add further state to the
equivalent libsemanage data structures and provide further processing in
the equivalent libsemanage interfaces, but not otherwise.  Especially as
the types are just pointers to opaque structures anyway and always
manipulated via get/set methods.

> I don't know - it's just a design thing to allow future change - Joshua?
> I'm not sure if the wrap functions add value, but at least they don't 
> cause any harm.

-- 
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 ] Fix memory leaks

[Ivan Gyurdiev]

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

The attached patch applies on top of the sepol patch (or at least..I 
hope it does - might complain about whitespace). It fixes memory leaks 
noticed by Steven Smalley, and removes the prototypes of two functions 
that are only used internally (there's a lot more, but this gets rid of 
two of them).


[-- Attachment #2: libsepol.fix.memory_leaks2.diff --]
[-- Type: text/x-patch, Size: 2489 bytes --]

diff -Naur libsepol.new/include/sepol/interfaces.h libsepol/include/sepol/interfaces.h
--- libsepol.new/include/sepol/interfaces.h	2005-09-20 02:55:29.000000000 -0400
+++ libsepol/include/sepol/interfaces.h	2005-09-20 16:34:11.000000000 -0400
@@ -5,13 +5,6 @@
 #include <sepol/iface_record.h>
 #include <stddef.h>
 
-/* Create a low level interface structure from
- * a high level representation */
-extern int sepol_iface_struct_create(
-	policydb_t* policydb,
-	ocontext_t** iface,
-	sepol_iface_t data);
-
 /* Get the current context mapping for this interface */
 extern int sepol_iface_get_context(
 	policydb_t* policydb,
diff -Naur libsepol.new/include/sepol/ports.h libsepol/include/sepol/ports.h
--- libsepol.new/include/sepol/ports.h	2005-09-14 11:44:44.000000000 -0400
+++ libsepol/include/sepol/ports.h	2005-09-20 16:34:18.000000000 -0400
@@ -5,12 +5,6 @@
 #include <sepol/port_record.h>
 #include <stddef.h>
 
-/* Create a port structure from high level representation */
-extern int sepol_port_struct_create(
-	policydb_t* policydb,
-	ocontext_t** port,
-	sepol_port_t data);
-
 /* Get the current context mapping
  * for this port. Returns 1 if no match, -1 on error, 0 on
  * success. The returned data is allocated on the heap */
diff -Naur libsepol.new/src/booleans.c libsepol/src/booleans.c
--- libsepol.new/src/booleans.c	2005-09-20 16:39:34.000000000 -0400
+++ libsepol/src/booleans.c	2005-09-20 16:39:24.000000000 -0400
@@ -33,6 +33,8 @@
 		DEBUG(__FUNCTION__, "illegal value %d for boolean %s\n", value, name);
 		goto err;
 	}
+
+	free(name);
 	datum->state = value;
 	return STATUS_SUCCESS;
 
diff -Naur libsepol.new/src/interfaces.c libsepol/src/interfaces.c
--- libsepol.new/src/interfaces.c	2005-09-20 02:55:29.000000000 -0400
+++ libsepol/src/interfaces.c	2005-09-20 16:33:11.000000000 -0400
@@ -51,6 +51,7 @@
 	DEBUG(__FUNCTION__, "out of memory\n");
 
 	err:
+	free(tmp_iface->u.name);
 	free(tmp_iface);
 	DEBUG(__FUNCTION__, "error creating interface structure\n");
 	return STATUS_ERR;
@@ -126,6 +127,7 @@
 
 	err:
 	DEBUG(__FUNCTION__, "error while loading interface %s\n", name);
+	free(iface->u.name);
 	free(iface);
 	return STATUS_ERR;
 }
diff -Naur libsepol.new/src/users.c libsepol/src/users.c
--- libsepol.new/src/users.c	2005-09-20 16:39:50.000000000 -0400
+++ libsepol/src/users.c	2005-09-20 16:39:06.000000000 -0400
@@ -102,6 +102,7 @@
 	if (sepol_user_load(policydb, user) < 0) 
 		goto err;
 
+	free(name);
 	return STATUS_SUCCESS;
 
 	omem:

RE: [ SEPOL/SEMANAGE ] Boolean record

[Karl MacMillan]

> -----Original Message-----
> From: owner-selinux@tycho.nsa.gov [mailto:owner-selinux@tycho.nsa.gov] On
> Behalf Of Ivan Gyurdiev
> Sent: Tuesday, September 20, 2005 4:16 PM
> To: Stephen Smalley
> Cc: selinux@tycho.nsa.gov; jbrindle@tresys.com
> Subject: Re: [ SEPOL/SEMANAGE ] Boolean record
> 
> 
> >- keep the shared libsepol, and have applications use its interfaces
> >directly.
> >
> To clarify, are you saying that we should have?
> 
> semanage_user_add(semanage_handle_t handle, sepol_user_key_t key,
> sepol_user_t data)
> semanage_user_del(semanage_handle_t handle, sepol_user_key_t key)
> semanage_user_modify(semanage_handle_t handle, sepol_user_t key,
> sepol_user_t data)
> 
> The sepol and semanage functionality is different - it's just the data
> structure that's shared.
> 

I agree that the duplication is strange, but I think that we should move all
of these into libsemanage - I'm looking through them now and I don't see
much reason for them to be in libsepol. This will only manage the files that
store the users, right? Is the only goal a shared parser for the files? Same
thing for ports.

> >That's what makes it seem so pointless.  If the wrap functions weren't
> >inlined and actually added some semantics around the libsepol
> >implementation, then they might make sense
> >
> >
> I don't know - it's just a design thing to allow future change - Joshua?
> I'm not sure if the wrap functions add value, but at least they don't
> cause any harm.
> 
> 

The inlined functions definitely don't make sense - the goals as I see them
are:

1) API/ABI stability
2) The ability to move between straight modules and the policy server.

The inlined functions don't meet this goal and moving things to libsepol
will make 2 impossible.

Karl

------
Karl MacMillan
Tresys Technology
http://www.tresys.com

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



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

Re: [ SEPOL/SEMANAGE ] Boolean record

[Ivan Gyurdiev]

>I agree that the duplication is strange, but I think that we should move all
>of these into libsemanage - I'm looking through them now and I don't see
>much reason for them to be in libsepol. This will only manage the files that
>store the users, right? Is the only goal a shared parser for the files? Same
>thing for ports.
>  
>
No - the semanage functionality is to extract data from a backend or 
write data to a backend, and to relay the corresponding objects to sepol 
to load (loading it itself breaks the encapsulation that I've been 
trying to create for sepol). The sepol functionality will be to actually 
load the data. Currently we're discussing the same data, hence the 
shared data structures, without modification. However, I'm not sure 
whether this will stay like that in the future - maybe we'll decide to 
store extra data in semanage that doesn't get passed down to the sepol 
layer. That's why I'm hesitant to use sepol data structures in semanage 
function headers.

>The inlined functions definitely don't make sense - the goals as I see them
>are:
>
>1) API/ABI stability
>2) The ability to move between straight modules and the policy server.
>
>The inlined functions don't meet this goal and moving things to libsepol
>will make 2 impossible.
>  
>
Can you clarify that point - I'm not sure I understand the problem. 
Perhaps it's an issue with inlined
functions that I'm not understanding - they're currently inlined for 
speed only, and that can easily be changed.
The point is to share the data structures with sepol, so the code 
doesn't have to be written twice.

--
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/SEMANAGE ] Boolean record

[Ivan Gyurdiev]

>2) The ability to move between straight modules and the policy server.
>
>The inlined functions don't meet this goal and moving things to libsepol
>will make 2 impossible.
>  
>
I don't understand what's preventing you from reimplementing those 
functions at any point in time, as long as the data structures are 
opaque (which they are). Yes, the functions are inlined, and the callers 
may decide that they'll always be equivalent to the sepol ones, but I 
think at that point the caller is not using the interface properly, and 
we shouldn't care.

Since there is no policy server right now, and there is a policydb, 
where the same data structures are already implemented, serving pretty 
much the same purpose (encapsulation), it seems easy to just make use of 
those functions. It also seems like you'll need to link to libsepol for 
a long time, to handle the policydb case even after the policy server is 
created.

Regardless, I don't care anymore - if you guys want to duplicate all the 
records into semanage, then that's fine with me - I can write a patch 
for it, if that is your decision. I think Steven's arguing for the exact 
opposite of that (unless I misunderstood) - use of sepol records 
directly. I just want some data structure that I can use in my record 
engine, regardless of where it came from - all I need for it is to fill 
out the record_table found in record_file.h.

Other issues: will you be merging the handle stuff soon? I need the 
handle to implement any kind of error reporting in my code (because 
otherwise I'd just be changing interfaces later). I think I'll likely 
submit a patch with the record engine stuff, and comment out all the 
DEBUG calls to be fixed later.

--
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/SEMANAGE ] Boolean record

[Stephen Smalley]

On Tue, 2005-09-20 at 17:07 -0400, Ivan Gyurdiev wrote:
> No - the semanage functionality is to extract data from a backend or 
> write data to a backend, and to relay the corresponding objects to sepol 
> to load (loading it itself breaks the encapsulation that I've been 
> trying to create for sepol). The sepol functionality will be to actually 
> load the data. Currently we're discussing the same data, hence the 
> shared data structures, without modification. However, I'm not sure 
> whether this will stay like that in the future - maybe we'll decide to 
> store extra data in semanage that doesn't get passed down to the sepol 
> layer. That's why I'm hesitant to use sepol data structures in semanage 
> function headers.

Then I think you need to hide this type aliasing and interface aliasing
within libsemanage, and not make it visible in its public headers.  And
I'm still not clear whether libsemanage should be exporting these
particular interfaces directly to its users, versus higher level
interfaces that are internally implemented in terms of the libsepol
primitives.  Do you expect libsemanage clients to be directly doing
things like semanage_port_compare?

-- 
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/SEMANAGE ] Boolean record

[Stephen Smalley]

On Tue, 2005-09-20 at 17:42 -0400, Ivan Gyurdiev wrote:
> Other issues: will you be merging the handle stuff soon? I need the 
> handle to implement any kind of error reporting in my code (because 
> otherwise I'd just be changing interfaces later). I think I'll likely 
> submit a patch with the record engine stuff, and comment out all the 
> DEBUG calls to be fixed later.

Changing interfaces later isn't a problem (yet).

-- 
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/SEMANAGE ] Boolean record

[Stephen Smalley]

On Tue, 2005-09-20 at 03:40 -0400, Ivan Gyurdiev wrote:
> The attached patches replace the boolinfo structure with a record.
> They also fix improper use of DEBUG, which would cause corrupt error 
> messages.

I've merged these patches for now, although I'm still not clear yet on
whether the relaying of records/interfaces from libsepol to libsemanage
is desirable/necessary.

-- 
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 ] Fix memory leaks

[Stephen Smalley]

On Tue, 2005-09-20 at 16:45 -0400, Ivan Gyurdiev wrote:
> The attached patch applies on top of the sepol patch (or at least..I 
> hope it does - might complain about whitespace). It fixes memory leaks 
> noticed by Steven Smalley, and removes the prototypes of two functions 
> that are only used internally (there's a lot more, but this gets rid of 
> two of them).

Thanks, merged.

-- 
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/SEMANAGE ] Boolean record

[Ivan Gyurdiev]

>Then I think you need to hide this type aliasing and interface aliasing
>within libsemanage, and not make it visible in its public headers.
>
Ok, the records will be un-inlined as requested.

I'll also be adding an avrule record soon. Another planned record is one 
for the file_context format. This one is clearly semanage-only, since 
the file_contexts specification does not live in policy. I'm not sure 
how that will be used at this point, but I think it will become 
important when we have to deal with labeling home directories. In 
general, it would be nice to be able to edit the file_contexts 
programmatically (maybe clear dependencies after module removal - not 
sure how tresys deals with things like that currently...probably better 
to use per-module file_contexts).

>  And
>I'm still not clear whether libsemanage should be exporting these
>particular interfaces directly to its users, versus higher level
>interfaces that are internally implemented in terms of the libsepol
>primitives.  Do you expect libsemanage clients to be directly doing
>things like semanage_port_compare?
>  
>
Probably not, but those interfaces don't do any harm - now you can check 
whether two opaque records represent the same data element, without 
knowing their structure.


--
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/SEMANAGE ] Boolean record

[Karl MacMillan]

> -----Original Message-----
> From: owner-selinux@tycho.nsa.gov [mailto:owner-selinux@tycho.nsa.gov] On
> Behalf Of Ivan Gyurdiev
> Sent: Tuesday, September 20, 2005 5:43 PM
> To: Karl MacMillan
> Cc: 'Stephen Smalley'; selinux@tycho.nsa.gov; jbrindle@tresys.com
> Subject: Re: [ SEPOL/SEMANAGE ] Boolean record
> 
> 
> >2) The ability to move between straight modules and the policy server.
> >
> >The inlined functions don't meet this goal and moving things to libsepol
> >will make 2 impossible.
> >
> >
> I don't understand what's preventing you from reimplementing those
> functions at any point in time, as long as the data structures are
> opaque (which they are). Yes, the functions are inlined, and the callers
> may decide that they'll always be equivalent to the sepol ones, but I
> think at that point the caller is not using the interface properly, and
> we shouldn't care.
> 
> Since there is no policy server right now, and there is a policydb,
> where the same data structures are already implemented, serving pretty
> much the same purpose (encapsulation), it seems easy to just make use of
> those functions. It also seems like you'll need to link to libsepol for
> a long time, to handle the policydb case even after the policy server is
> created.
> 

Just a note - there is a working policy server right now
(sepolicy-server.sf.net has the latest release). That is why these
functions, at the very least, need to be indirected through the
semanage_handle_t function pointers so different versions can be plugged in
for the remote implementation.

Karl

------
Karl MacMillan
Tresys Technology
http://www.tresys.com

> Regardless, I don't care anymore - if you guys want to duplicate all the
> records into semanage, then that's fine with me - I can write a patch
> for it, if that is your decision. I think Steven's arguing for the exact
> opposite of that (unless I misunderstood) - use of sepol records
> directly. I just want some data structure that I can use in my record
> engine, regardless of where it came from - all I need for it is to fill
> out the record_table found in record_file.h.
> 
> Other issues: will you be merging the handle stuff soon? I need the
> handle to implement any kind of error reporting in my code (because
> otherwise I'd just be changing interfaces later). I think I'll likely
> submit a patch with the record engine stuff, and comment out all the
> DEBUG calls to be fixed later.
> 
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov
> with
> the words "unsubscribe selinux" without quotes as the message.



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

RE: [ SEPOL/SEMANAGE ] Boolean record

[Stephen Smalley]

On Wed, 2005-09-21 at 13:48 -0400, Karl MacMillan wrote:
> Just a note - there is a working policy server right now
> (sepolicy-server.sf.net has the latest release). That is why these
> functions, at the very least, need to be indirected through the
> semanage_handle_t function pointers so different versions can be plugged in
> for the remote implementation.

Where is semanage_handle_t defined?  Or better yet, submit a patch
adding the definition to libsemanage now.

-- 
Stephen Smalley
National Security Agency


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

RE: [ SEPOL/SEMANAGE ] Boolean record

[Stephen Smalley]

On Wed, 2005-09-21 at 13:48 -0400, Karl MacMillan wrote:
> Just a note - there is a working policy server right now
> (sepolicy-server.sf.net has the latest release). That is why these
> functions, at the very least, need to be indirected through the
> semanage_handle_t function pointers so different versions can be plugged in
> for the remote implementation.

Last release there that I see was April.  And the CVS looks dead.

-- 
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/SEMANAGE ] Boolean record

[Karl MacMillan]

> -----Original Message-----
> From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
> Sent: Wednesday, September 21, 2005 1:54 PM
> To: Karl MacMillan
> Cc: 'Ivan Gyurdiev'; selinux@tycho.nsa.gov; jbrindle@tresys.com
> Subject: RE: [ SEPOL/SEMANAGE ] Boolean record
> 
> On Wed, 2005-09-21 at 13:48 -0400, Karl MacMillan wrote:
> > Just a note - there is a working policy server right now
> > (sepolicy-server.sf.net has the latest release). That is why these
> > functions, at the very least, need to be indirected through the
> > semanage_handle_t function pointers so different versions can be plugged
> in
> > for the remote implementation.
> 
> Last release there that I see was April.  And the CVS looks dead.
>

A new release is pending the completion of the libsemanage changes.

Karl

------
Karl MacMillan
Tresys Technology
http://www.tresys.com

> --
> 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/SEMANAGE ] Boolean record

[Karl MacMillan]

> -----Original Message-----
> From: Ivan Gyurdiev [mailto:ivg2@cornell.edu]
> Sent: Wednesday, September 21, 2005 2:37 PM
> To: Karl MacMillan
> Cc: 'Stephen Smalley'; selinux@tycho.nsa.gov; jbrindle@tresys.com
> Subject: Re: [ SEPOL/SEMANAGE ] Boolean record
> 
> 
> >Just a note - there is a working policy server right now
> >(sepolicy-server.sf.net has the latest release). That is why these
> >functions, at the very least, need to be indirected through the
> >semanage_handle_t function pointers so different versions can be plugged
> in
> >for the remote implementation.
> >
> >
> These functions should be implemented exactly the same way, regardless
> of backend of the policy.
> Those are data structure wrappers. They provide virtually no
> functionality other than accessors for the data
> in the structure.
> 

Not certain what you mean - in the case of the policy server, the client
might be running on a different machine than the server. That means that
these functions would have to do some form of RPC using a wire protocol. You
can look at the old policy server snapshot to see this.

Karl

------
Karl MacMillan
Tresys Technology
http://www.tresys.com




--
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/SEMANAGE ] Boolean record

[Ivan Gyurdiev]

>Just a note - there is a working policy server right now
>(sepolicy-server.sf.net has the latest release). That is why these
>functions, at the very least, need to be indirected through the
>semanage_handle_t function pointers so different versions can be plugged in
>for the remote implementation.
>  
>
These functions should be implemented exactly the same way, regardless 
of backend of the policy.
Those are data structure wrappers. They provide virtually no 
functionality other than accessors for the data
in the structure.




--
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/SEMANAGE ] Boolean record

[Ivan Gyurdiev]

>Not certain what you mean - in the case of the policy server, the client
>might be running on a different machine than the server. That means that
>these functions would have to do some form of RPC using a wire protocol. You
>can look at the old policy server snapshot to see this.
>  
>
I mean... the functions in *_record.h pack and unpack records, and don't 
do anything else of significance - they don't retrieve the record, or 
parse the record, or anything. As such, I don't see why you'd be using 
RPC on them - there's no reason for a client to be invoking those 
functions on a different machine.


 






--
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/SEMANAGE ] Boolean record

[Karl MacMillan]

> -----Original Message-----
> From: owner-selinux@tycho.nsa.gov [mailto:owner-selinux@tycho.nsa.gov] On
> Behalf Of Ivan Gyurdiev
> Sent: Wednesday, September 21, 2005 9:50 PM
> To: Karl MacMillan
> Cc: 'Stephen Smalley'; selinux@tycho.nsa.gov; jbrindle@tresys.com
> Subject: Re: [ SEPOL/SEMANAGE ] Boolean record
> 
> 
> >Not certain what you mean - in the case of the policy server, the client
> >might be running on a different machine than the server. That means that
> >these functions would have to do some form of RPC using a wire protocol.
> You
> >can look at the old policy server snapshot to see this.
> >
> >
> I mean... the functions in *_record.h pack and unpack records, and don't
> do anything else of significance - they don't retrieve the record, or
> parse the record, or anything. As such, I don't see why you'd be using
> RPC on them - there's no reason for a client to be invoking those
> functions on a different machine.
> 
> 

There are 2 reasons:

1) To enforce access control on policy change, the policy server needs to
run in a separate process (not necessarily on another machine, but that
works as well). That means IPC/RPC by the client.

2) Eventually the goal is to be able to change policy on a group of machines
together. This likely requires running the client on separate machine from
the policy server.

Karl

------
Karl MacMillan
Tresys Technology
http://www.tresys.com

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



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