[SEPOL] Remove defrole from sepol

[SEPOL] Remove defrole from sepol

[Ivan Gyurdiev]

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

This patch removes defrole from sepol, because it does not belong there, 
and it's just plain wrong. The default role is not preserved in the 
binary policy - therefore it can only exist in semanage (unless we 
change the policy format to contain it). This simplifies user_record.c.

It also updates del_role to have a void return type, as it can no longer 
fail.

Now we need to add the labeling prefix back into semanage somehow.


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

diff -Naurp --exclude CVS --exclude ChangeLog --exclude VERSION --exclude 'booleans_kernel.*' --exclude 'database_pserver.*' old/libsemanage/include/semanage/user_record.h new/libsemanage/include/semanage/user_record.h
--- old/libsemanage/include/semanage/user_record.h	2005-11-08 09:32:57.000000000 -0500
+++ new/libsemanage/include/semanage/user_record.h	2005-11-18 19:55:14.000000000 -0500
@@ -70,7 +70,7 @@ extern int semanage_user_add_role(
 	semanage_user_t* user, 
 	const char* role);
 
-extern int semanage_user_del_role(
+extern void semanage_user_del_role(
 	semanage_handle_t* handle,
 	semanage_user_t* user, 
 	const char* role);
diff -Naurp --exclude CVS --exclude ChangeLog --exclude VERSION --exclude 'booleans_kernel.*' --exclude 'database_pserver.*' old/libsemanage/src/semanageswig_wrap.c new/libsemanage/src/semanageswig_wrap.c
--- old/libsemanage/src/semanageswig_wrap.c	2005-11-15 08:06:18.000000000 -0500
+++ new/libsemanage/src/semanageswig_wrap.c	2005-11-19 00:39:01.000000000 -0500
@@ -1680,7 +1680,7 @@ int semanage_user_set_mlsrange(semanage_
 int semanage_user_get_num_roles(semanage_user_t *);
 char const *semanage_user_get_defrole(semanage_user_t *);
 int semanage_user_add_role(semanage_handle_t *,semanage_user_t *,char const *);
-int semanage_user_del_role(semanage_handle_t *,semanage_user_t *,char const *);
+void semanage_user_del_role(semanage_handle_t *,semanage_user_t *,char const *);
 int semanage_user_has_role(semanage_user_t *,char const *);
 int semanage_user_set_defrole(semanage_handle_t *,semanage_user_t *,char const *);
 int semanage_user_get_roles(semanage_handle_t *,semanage_user_t *,char const ***,size_t *);
@@ -3292,7 +3292,6 @@ static PyObject *_wrap_semanage_user_del
     semanage_handle_t *arg1 = (semanage_handle_t *) 0 ;
     semanage_user_t *arg2 = (semanage_user_t *) 0 ;
     char *arg3 = (char *) 0 ;
-    int result;
     PyObject * obj0 = 0 ;
     PyObject * obj1 = 0 ;
     PyObject * obj2 = 0 ;
@@ -3305,11 +3304,9 @@ static PyObject *_wrap_semanage_user_del
     if (!SWIG_AsCharPtr(obj2, (char**)&arg3)) {
         SWIG_arg_fail(3);SWIG_fail;
     }
-    result = (int)semanage_user_del_role(arg1,arg2,(char const *)arg3);
-    
-    {
-        resultobj = SWIG_From_int((int)(result)); 
-    }
+    semanage_user_del_role(arg1,arg2,(char const *)arg3);
+
+    Py_INCREF(Py_None); resultobj = Py_None;
     return resultobj;
     fail:
     return NULL;
diff -Naurp --exclude CVS --exclude ChangeLog --exclude VERSION --exclude 'booleans_kernel.*' --exclude 'database_pserver.*' old/libsemanage/src/user_record.c new/libsemanage/src/user_record.c
--- old/libsemanage/src/user_record.c	2005-11-08 09:32:57.000000000 -0500
+++ new/libsemanage/src/user_record.c	2005-11-19 00:37:59.000000000 -0500
@@ -12,9 +12,11 @@ typedef semanage_user_t record_t;
 typedef semanage_user_key_t record_key_t;
 #define DBASE_RECORD_DEFINED
 
+#include <stdlib.h>
 #include <stddef.h>
 #include "handle.h"
 #include "database.h"
+#include "debug.h"
 
 /* Key */
 int semanage_user_key_create(
@@ -110,7 +112,9 @@ int semanage_user_get_num_roles(
 const char* semanage_user_get_defrole(
 	semanage_user_t* user) {
 
-	return sepol_user_get_defrole(user);
+	/* FIXME: stub */
+	user = NULL;
+	return "";
 }
 hidden_def(semanage_user_get_defrole)
 
@@ -123,12 +127,12 @@ int semanage_user_add_role(
 }
 hidden_def(semanage_user_add_role)
 
-int semanage_user_del_role(
+void semanage_user_del_role(
 	semanage_handle_t* handle,
 	semanage_user_t* user, 
 	const char* role) {
 
-	return sepol_user_del_role(handle->sepolh, user, role);
+	sepol_user_del_role(handle->sepolh, user, role);
 }
 
 int semanage_user_has_role(
@@ -143,7 +147,11 @@ int semanage_user_set_defrole(
 	semanage_user_t* user, 
 	const char* role) {
 
-	return sepol_user_set_defrole(handle->sepolh, user, role);
+	/* FIXME: stub */
+	handle = NULL;
+	user = NULL;
+	role = NULL;
+	return STATUS_ERR;
 }
 hidden_def(semanage_user_set_defrole)
 
diff -Naurp --exclude CVS --exclude ChangeLog --exclude VERSION --exclude 'booleans_kernel.*' --exclude 'database_pserver.*' old/libsepol/include/sepol/user_record.h new/libsepol/include/sepol/user_record.h
--- old/libsepol/include/sepol/user_record.h	2005-10-31 11:09:39.000000000 -0500
+++ new/libsepol/include/sepol/user_record.h	2005-11-18 19:51:51.000000000 -0500
@@ -61,15 +61,12 @@ extern int sepol_user_set_mlsrange(
 extern int sepol_user_get_num_roles(
 	sepol_user_t* user);
 
-extern const char* sepol_user_get_defrole(
-	sepol_user_t* user);
-
 extern int sepol_user_add_role(
 	sepol_handle_t* handle,
 	sepol_user_t* user, 
 	const char* role);
 
-extern int sepol_user_del_role(
+extern void sepol_user_del_role(
 	sepol_handle_t* handle,
 	sepol_user_t* user, 
 	const char* role);
@@ -78,11 +75,6 @@ extern int sepol_user_has_role(
 	sepol_user_t* user, 
 	const char* role);
 
-extern int sepol_user_set_defrole(
-	sepol_handle_t* handle,
-	sepol_user_t* user, 
-	const char* role);
-
 extern int sepol_user_get_roles(
 	sepol_handle_t* handle,
 	sepol_user_t* user,
diff -Naurp --exclude CVS --exclude ChangeLog --exclude VERSION --exclude 'booleans_kernel.*' --exclude 'database_pserver.*' old/libsepol/src/user_internal.h new/libsepol/src/user_internal.h
--- old/libsepol/src/user_internal.h	2005-11-01 17:32:59.000000000 -0500
+++ new/libsepol/src/user_internal.h	2005-11-18 19:52:24.000000000 -0500
@@ -11,7 +11,6 @@ hidden_proto(sepol_user_get_roles)
 hidden_proto(sepol_user_has_role)
 hidden_proto(sepol_user_key_create)
 hidden_proto(sepol_user_key_unpack)
-hidden_proto(sepol_user_set_defrole)
 hidden_proto(sepol_user_set_mlslevel)
 hidden_proto(sepol_user_set_mlsrange)
 hidden_proto(sepol_user_set_name)
diff -Naurp --exclude CVS --exclude ChangeLog --exclude VERSION --exclude 'booleans_kernel.*' --exclude 'database_pserver.*' old/libsepol/src/user_record.c new/libsepol/src/user_record.c
--- old/libsepol/src/user_record.c	2005-11-01 17:32:59.000000000 -0500
+++ new/libsepol/src/user_record.c	2005-11-18 19:51:17.000000000 -0500
@@ -21,9 +21,6 @@ struct sepol_user {
 
 	/* The number of roles */
 	size_t num_roles;
-
-	/* The default role */
-	char* def_role;
 };
 
 struct sepol_user_key {
@@ -158,44 +155,33 @@ int sepol_user_get_num_roles(sepol_user_
 	return user->num_roles;
 }
 
-const char* sepol_user_get_defrole(sepol_user_t* user) {
-	 return (user->def_role == NULL)? NULL : user->def_role;
-}
-
 int sepol_user_add_role(
 	sepol_handle_t* handle,
 	sepol_user_t* user, 
 	const char* role) {
 
 	char* role_cp;
-	char* role_cp2;
 	char** roles_realloc;
 
 	if (sepol_user_has_role(user, role))
 		return STATUS_SUCCESS;
 
 	role_cp = strdup(role);
-	role_cp2 = strdup(role); 
 	roles_realloc = realloc(user->roles, 
 		sizeof(char*) * (user->num_roles + 1));
 
-	if (!role_cp || !role_cp2 || !roles_realloc) 
+	if (!role_cp || !roles_realloc) 
 		goto omem;
 
 	user->num_roles++;
 	user->roles = roles_realloc;
 	user->roles[user->num_roles - 1] = role_cp;
-	if (user->def_role == NULL)
-		user->def_role = role_cp2;
-	else
-		free(role_cp2);
 
         return STATUS_SUCCESS;
 
 	omem:
 	ERR(handle, "out of memory, could not add role %s", role);
 	free(role_cp);
-	free(role_cp2);
 	free(roles_realloc);
 	return STATUS_ERR;
 }
@@ -219,7 +205,6 @@ int sepol_user_set_roles(
 
 	size_t i;
 	char** tmp_roles = NULL;
-	char* tmp_def_role = NULL;
 
 	if (num_roles > 0) {
 
@@ -233,21 +218,14 @@ int sepol_user_set_roles(
 			if (!tmp_roles[i])
 				goto omem;	
 		}
-
-		tmp_def_role = strdup(tmp_roles[0]);
-		if (!tmp_def_role) 
-			goto omem;
 	}
 	
 	/* Apply other changes */
 	for (i = 0; i < user->num_roles; i++)
 		free(user->roles[i]);
 	free(user->roles);
-	free(user->def_role);
 	user->roles = tmp_roles;
 	user->num_roles = num_roles;
-	user->def_role = tmp_def_role;
-
 	return STATUS_SUCCESS;
 
 	omem:
@@ -262,7 +240,6 @@ int sepol_user_set_roles(
 		}
 	}
 	free(tmp_roles);
-	free(tmp_def_role);
 	return STATUS_ERR;	
 }
 
@@ -293,73 +270,22 @@ int sepol_user_get_roles(
 }
 hidden_def(sepol_user_get_roles)
 
-int sepol_user_del_role(
+void sepol_user_del_role(
 	sepol_handle_t* handle,	
 	sepol_user_t* user, 
 	const char* role) {
 
-	int change_defrole = 0;
-	char* tmp_defrole = NULL;
 	size_t i;
-
 	for (i = 0; i < user->num_roles; i++) {
 		if (!strcmp(user->roles[i], role)) {
-
-			/* Will replace default role */
-			if (user->num_roles > 1 && !strcmp(user->def_role, role))  {
-				tmp_defrole = strdup(user->roles[0]);
-				if (!tmp_defrole) {
-					ERR(handle, 
-						"out of memory, could not allocate "
-						"new default role");
-					return STATUS_ERR;
-				}
-				change_defrole = 1;
-			}
-
-			/* Apply changes */
 			free(user->roles[i]);
+                        user->roles[i] = NULL;
 			user->roles[i] = user->roles[user->num_roles-1];			
 			user->num_roles--;
-			if (change_defrole) {
-				free(user->def_role);
-				user->def_role = tmp_defrole;
-			}
-
-			return STATUS_SUCCESS;
 		}
 	}
-
-	return STATUS_SUCCESS;
 }
 
-int sepol_user_set_defrole(
-	sepol_handle_t* handle,
-	sepol_user_t* user, 
-	const char* role) {
-
-	char* tmp_defrole = strdup(role);
-	if (!tmp_defrole)
-		goto omem;
-
-	if (sepol_user_add_role(handle, user, role) < 0)
-		goto err;
-
-	free(user->def_role);
-	user->def_role = tmp_defrole;
-	return STATUS_SUCCESS;
-
-	omem:
-	ERR(handle, "out of memory");
-	
-	err:
-	free(tmp_defrole);
-	ERR(handle, "could not set default role for %s to %s", 
-		user->name, role);
-	return STATUS_ERR;
-}
-hidden_def(sepol_user_set_defrole)
-
 /* Create */
 int sepol_user_create(
 	sepol_handle_t* handle,
@@ -374,7 +300,6 @@ int sepol_user_create(
 	}
 
         user->roles = NULL;
-        user->def_role = NULL;
         user->num_roles = 0;
         user->name = NULL;
 	user->mls_level = NULL;
@@ -405,9 +330,6 @@ int sepol_user_clone(
 			goto err;
 	}
 
-	if (sepol_user_set_defrole(handle, new_user, user->def_role) < 0)
-		goto err;	
-
 	if (user->mls_level &&
 	   (sepol_user_set_mlslevel(handle, new_user, user->mls_level) < 0))
 		goto err;
@@ -435,7 +357,6 @@ void sepol_user_free(sepol_user_t* user)
 	free(user->name);
 	for (i = 0; i < user->num_roles; i++)
 		free(user->roles[i]);
-	free(user->def_role);
 	free(user->roles);
 	free(user->mls_level);
 	free(user->mls_range);

Re: [SEPOL] Remove defrole from sepol

[Ivan Gyurdiev]

> Now we need to add the labeling prefix back into semanage somehow.
What APIs are needed here?

I have a patch that adds another record called semanage_user_aux_t (User 
Auxiliary Data), defined as data keyed on the user that stays out of 
sepol. It currently provides a method to get the defrole. It's backed by 
a flat file formatted: (name:defrole name:defrole ...)

If we have user_aux.local, and user_aux.system, how does the policy rpm 
install the system users - does it need modification APIs for that? Is 
there a third file that combines the local and system one 
(rebuild-from-scratch equivalent for flat files), or do I need to 
implement combined query/exists/iterate/..* operations (with list() and 
iterate() checking for duplicates, and giving preference to a local entry).

=========

P.S. I am suspicious that too many record/dbase functions are currently 
exposed in sepol or semanage - will have to take a look again to make 
sure they're all needed (and none are missing... because I see some that 
aren't there, like the clone function (which likely should likely stay 
hidden)).

--
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] Remove defrole from sepol

[Ivan Gyurdiev]

>
> It also updates del_role to have a void return type, as it can no 
> longer fail.
Hmm,  I also need to remove the handle argument.

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

Re: [SEPOL] Remove defrole from sepol

[Ivan Gyurdiev]

I'm starting to question the need for this interface at all... it's an 
interface for a very narrow user base - genhomedircon... which is 
probably a mistake. I would prefer genhomedircon to find its way into 
libsemanage, which is its only user (does it have another one?). Then 
there would be no reason for an external interface for default roles and 
hacks to move genhomedircon before one lock is released, and after the 
other is released, and things like that would not be necessary. 
Genhomedircon encapsulates an implementation detail of user/seuser 
updates. Is there another reason for it being outside libsemanage, other 
than python being easy to work with?



--
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] Remove defrole from sepol

[Joshua Brindle]

Ivan Gyurdiev wrote:
> I'm starting to question the need for this interface at all... it's an 
> interface for a very narrow user base - genhomedircon... which is 
> probably a mistake. I would prefer genhomedircon to find its way into 
> libsemanage, which is its only user (does it have another one?). 
the semanage tool Dan is writing could use it, to determine if a level 
should be set, or it could just rely on getting an error back if you try 
to set a level and it is a non mls system.


> there would be no reason for an external interface for default roles and 
> hacks to move genhomedircon before one lock is released, and after the 
> other is released, and things like that would not be necessary. 
genhomedircon is a reader, it is using resources outside of the module 
store and is not confined to the sandbox. I don't know that it should be 
inside the transaction.

> Genhomedircon encapsulates an implementation detail of user/seuser 
> updates. Is there another reason for it being outside libsemanage, other 
> than python being easy to work with?
>
Mainly that its already done and works. We really do have higher 
priority things than reimplementing genhomedircon in C to put in 
libsemanage.

--
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] Remove defrole from sepol

[Ivan Gyurdiev]

>> I'm starting to question the need for this interface at all... it's 
>> an interface for a very narrow user base - genhomedircon... which is 
>> probably a mistake. I would prefer genhomedircon to find its way into 
>> libsemanage, which is its only user (does it have another one?). 
> the semanage tool Dan is writing could use it, to determine if a level 
> should be set, or it could just rely on getting an error back if you 
> try to set a level and it is a non mls system.
I'm confused... it needs genhomedircon, and not the library?
>> there would be no reason for an external interface for default roles 
>> and hacks to move genhomedircon before one lock is released, and 
>> after the other is released, and things like that would not be 
>> necessary. 
> genhomedircon is a reader, it is using resources outside of the module 
> store and is not confined to the sandbox. I don't know that it should 
> be inside the transaction.
Readers should be inside the transaction to guard against race 
condition. You mentioned commit numbers, and I pointed out I don't use 
them yet - and I don't see how they're a win over using a transaction.
>> Genhomedircon encapsulates an implementation detail of user/seuser 
>> updates. Is there another reason for it being outside libsemanage, 
>> other than python being easy to work with?
>>
> Mainly that its already done and works. We really do have higher 
> priority things than reimplementing genhomedircon in C to put in 
> libsemanage.
Maybe... unless it results in libsemanage APIs that are going to be 
removed later.  What's your opinion of what this API should look like - 
opaque object that can be expanded for other data (aux_info), or prefix 
only? How would system vs local work - see other questions in the thread.


--
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] Remove defrole from sepol

[Joshua Brindle]

Ivan Gyurdiev wrote:
> 
>>> I'm starting to question the need for this interface at all... it's 
>>> an interface for a very narrow user base - genhomedircon... which is 
>>> probably a mistake. I would prefer genhomedircon to find its way into 
>>> libsemanage, which is its only user (does it have another one?). 
>>
>> the semanage tool Dan is writing could use it, to determine if a level 
>> should be set, or it could just rely on getting an error back if you 
>> try to set a level and it is a non mls system.
> 
> I'm confused... it needs genhomedircon, and not the library?
> 
which interface were you refering to? the semanage tool will need to 
modify most entry types.

>>> there would be no reason for an external interface for default roles 
>>> and hacks to move genhomedircon before one lock is released, and 
>>> after the other is released, and things like that would not be 
>>> necessary. 
>>
>> genhomedircon is a reader, it is using resources outside of the module 
>> store and is not confined to the sandbox. I don't know that it should 
>> be inside the transaction.
> 
> Readers should be inside the transaction to guard against race 
> condition. You mentioned commit numbers, and I pointed out I don't use 
> them yet - and I don't see how they're a win over using a transaction.
> 
You should be, this should certainly be a higher priority to fix than 
reimplementing a working tool.

>>> Genhomedircon encapsulates an implementation detail of user/seuser 
>>> updates. Is there another reason for it being outside libsemanage, 
>>> other than python being easy to work with?
>>>
>> Mainly that its already done and works. We really do have higher 
>> priority things than reimplementing genhomedircon in C to put in 
>> libsemanage.
> 
> Maybe... unless it results in libsemanage APIs that are going to be 
> removed later.  What's your opinion of what this API should look like - 
> opaque object that can be expanded for other data (aux_info), or prefix 
> only? How would system vs local work - see other questions in the thread.
> 
> 

this is the reason semanage_user, etc are opaque. you can add a field 
and accessor to it without interupting anything else. Therefore you 
should just export a new piece of data from whatever type needs it, and 
if something else is needed later the same thing can be done.

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

Re: [SEPOL] Remove defrole from sepol

[Ivan Gyurdiev]

>>>> I'm starting to question the need for this interface at all... it's 
>>>> an interface for a very narrow user base - genhomedircon... which 
>>>> is probably a mistake. I would prefer genhomedircon to find its way 
>>>> into libsemanage, which is its only user (does it have another one?). 
>>>
>>> the semanage tool Dan is writing could use it, to determine if a 
>>> level should be set, or it could just rely on getting an error back 
>>> if you try to set a level and it is a non mls system.
>>
>> I'm confused... it needs genhomedircon, and not the library?
>>
> which interface were you refering to? the semanage tool will need to 
> modify most entry types.
I was referring to the interface to export a default role for 
genhomedircon. It's broken right now, but I'm re-writing it as a general 
"Auxiliary Data" object... but if there is no other auxiliary data than 
the labeling prefix/defrole, and the only user is genhomedircon, it's 
not clear whether a new interface should be written.
>>>> there would be no reason for an external interface for default 
>>>> roles and hacks to move genhomedircon before one lock is released, 
>>>> and after the other is released, and things like that would not be 
>>>> necessary. 
>>>
>>> genhomedircon is a reader, it is using resources outside of the 
>>> module store and is not confined to the sandbox. I don't know that 
>>> it should be inside the transaction.
>>
>> Readers should be inside the transaction to guard against race 
>> condition. You mentioned commit numbers, and I pointed out I don't 
>> use them yet - and I don't see how they're a win over using a 
>> transaction.
>>
> You should be, this should certainly be a higher priority to fix than 
> reimplementing a working tool.
Why would I want to be checking commit numbers in the library client? 
How would failure be handled - re-try enough times until it works? Why 
is this better than using a transaction?

>>>> Genhomedircon encapsulates an implementation detail of user/seuser 
>>>> updates. Is there another reason for it being outside libsemanage, 
>>>> other than python being easy to work with?
>>>>
>>> Mainly that its already done and works. We really do have higher 
>>> priority things than reimplementing genhomedircon in C to put in 
>>> libsemanage.
>>
>> Maybe... unless it results in libsemanage APIs that are going to be 
>> removed later.  What's your opinion of what this API should look like 
>> - opaque object that can be expanded for other data (aux_info), or 
>> prefix only? How would system vs local work - see other questions in 
>> the thread.
>>
>>
>
> this is the reason semanage_user, etc are opaque. you can add a field 
> and accessor to it without interupting anything else. Therefore you 
> should just export a new piece of data from whatever type needs it, 
> and if something else is needed later the same thing can be done.
Speaking of which... this doesn't work as nicely as you say. 
Semantically this default role/labeling prefix thing belongs in the user 
record, and not anywhere else. There's no reason for making a new record 
for "auxiliary data". It's keyed on the user, because it should be in 
the user record. The only reason I'm considering adding a new record 
type, is because the policydb backend does not support marking which 
role is default, and writing functions like iterate() over two different 
backends simultaneously (file for defrole, policy for other info) is 
just wrong.... it's an implementation-driven interface, and not the 
other way around, which I don't like.



--
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] Remove defrole from sepol

[Ivan Gyurdiev]

>
>>>>> I'm starting to question the need for this interface at all... 
>>>>> it's an interface for a very narrow user base - genhomedircon... 
>>>>> which is probably a mistake. I would prefer genhomedircon to find 
>>>>> its way into libsemanage, which is its only user (does it have 
>>>>> another one?). 
>>>>
>>>> the semanage tool Dan is writing could use it, to determine if a 
>>>> level should be set, or it could just rely on getting an error back 
>>>> if you try to set a level and it is a non mls system.
>>>
>>> I'm confused... it needs genhomedircon, and not the library?
>>>
>> which interface were you refering to? the semanage tool will need to 
>> modify most entry types.
> I was referring to the interface to export a default role for 
> genhomedircon. It's broken right now, but I'm re-writing it as a 
> general "Auxiliary Data" object... but if there is no other auxiliary 
> data than the labeling prefix/defrole, and the only user is 
> genhomedircon, it's not clear whether a new interface should be written.
Well I guess we'll need some kind of interface to configure labeling... 
whether genhomedircon is inside or outside libsemanage. I just don't 
like how I'm forced into adding another record which shouldn't be 
necessary (because the alternative of adding info to the user record, 
and having it backed by both policy and file is worse)... and now have 
to add code to merge a pair of files in the list() and iterate() functions.


--
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] Remove defrole from sepol

[Joshua Brindle]

Ivan Gyurdiev wrote:
> 
>>
>>>>>> I'm starting to question the need for this interface at all... 
>>>>>> it's an interface for a very narrow user base - genhomedircon... 
>>>>>> which is probably a mistake. I would prefer genhomedircon to find 
>>>>>> its way into libsemanage, which is its only user (does it have 
>>>>>> another one?). 
>>>>>
>>>>>
>>>>> the semanage tool Dan is writing could use it, to determine if a 
>>>>> level should be set, or it could just rely on getting an error back 
>>>>> if you try to set a level and it is a non mls system.
>>>>
>>>>
>>>> I'm confused... it needs genhomedircon, and not the library?
>>>>
>>> which interface were you refering to? the semanage tool will need to 
>>> modify most entry types.
>>
>> I was referring to the interface to export a default role for 
>> genhomedircon. It's broken right now, but I'm re-writing it as a 
>> general "Auxiliary Data" object... but if there is no other auxiliary 
>> data than the labeling prefix/defrole, and the only user is 
>> genhomedircon, it's not clear whether a new interface should be written.
> 
> Well I guess we'll need some kind of interface to configure labeling... 
> whether genhomedircon is inside or outside libsemanage. I just don't 
> like how I'm forced into adding another record which shouldn't be 
> necessary (because the alternative of adding info to the user record, 
> and having it backed by both policy and file is worse)... and now have 
> to add code to merge a pair of files in the list() and iterate() functions.
> 
> 
This is what databases are all about, relational ones anyway...


--
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] Remove defrole from sepol

[Joshua Brindle]

Ivan Gyurdiev wrote:
> 
>>> I'm starting to question the need for this interface at all... it's 
>>> an interface for a very narrow user base - genhomedircon... which is 
>>> probably a mistake. I would prefer genhomedircon to find its way into 
>>> libsemanage, which is its only user (does it have another one?). 
>>
>> the semanage tool Dan is writing could use it, to determine if a level 
>> should be set, or it could just rely on getting an error back if you 
>> try to set a level and it is a non mls system.
> 
> I'm confused... it needs genhomedircon, and not the library?
> 
>>> there would be no reason for an external interface for default roles 
>>> and hacks to move genhomedircon before one lock is released, and 
>>> after the other is released, and things like that would not be 
>>> necessary. 
>>
>> genhomedircon is a reader, it is using resources outside of the module 
>> store and is not confined to the sandbox. I don't know that it should 
>> be inside the transaction.
> 
> Readers should be inside the transaction to guard against race 
> condition. You mentioned commit numbers, and I pointed out I don't use 
> them yet - and I don't see how they're a win over using a transaction.
> 
Copying the whole store to read a boolean is pretty bad. If the user 
(app) intends to make a modification they can start a transaction before 
  querying, to ensure consistency but a standard reader should not need 
to do this. Why copy a whole directory over to do 2 queries when 
genhomedircon can just compare 2 numbers?

>>> Genhomedircon encapsulates an implementation detail of user/seuser 
>>> updates. Is there another reason for it being outside libsemanage, 
>>> other than python being easy to work with?
>>>
>> Mainly that its already done and works. We really do have higher 
>> priority things than reimplementing genhomedircon in C to put in 
>> libsemanage.
> 
> Maybe... unless it results in libsemanage APIs that are going to be 
> removed later.  What's your opinion of what this API should look like - 
> opaque object that can be expanded for other data (aux_info), or prefix 
> only? How would system vs local work - see other questions in the thread.
> 
> 


--
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] Remove defrole from sepol

[Ivan Gyurdiev]

>
>>
>> Readers should be inside the transaction to guard against race 
>> condition. You mentioned commit numbers, and I pointed out I don't 
>> use them yet - and I don't see how they're a win over using a 
>> transaction.
>>
> Copying the whole store to read a boolean is pretty bad. If the user 
> (app) intends to make a modification they can start a transaction 
> before  querying, to ensure consistency but a standard reader should 
> not need to do this. Why copy a whole directory over to do 2 queries 
> when genhomedircon can just compare 2 numbers?

..because then it has to deal with the failure case of when the two 
numbers are different. I guess in the general case, maybe the reader 
doesn't want to handle the failure case, so it makes sense to add commit 
numbers.


--
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] Remove defrole from sepol

[Joshua Brindle]

Ivan Gyurdiev wrote:
>>
>>>
>>> Readers should be inside the transaction to guard against race 
>>> condition. You mentioned commit numbers, and I pointed out I don't 
>>> use them yet - and I don't see how they're a win over using a 
>>> transaction.
>>>
>> Copying the whole store to read a boolean is pretty bad. If the user 
>> (app) intends to make a modification they can start a transaction 
>> before  querying, to ensure consistency but a standard reader should 
>> not need to do this. Why copy a whole directory over to do 2 queries 
>> when genhomedircon can just compare 2 numbers?
> 
> 
> ..because then it has to deal with the failure case of when the two 
> numbers are different. I guess in the general case, maybe the reader 
> doesn't want to handle the failure case, so it makes sense to add commit 
> numbers.
> 
> 
Either way you have to handle failure to obtain the lock. Transaction
locks are more likely to be held longer since they are given up
discretionally, whereas query locks are only held for the duration of
that query.

--
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] Remove defrole from sepol

[Stephen Smalley]

On Sat, 2005-11-19 at 00:50 -0500, Ivan Gyurdiev wrote:
> This patch removes defrole from sepol, because it does not belong there, 
> and it's just plain wrong. The default role is not preserved in the 
> binary policy - therefore it can only exist in semanage (unless we 
> change the policy format to contain it). This simplifies user_record.c.
> 
> It also updates del_role to have a void return type, as it can no longer 
> fail.
> 
> Now we need to add the labeling prefix back into semanage somehow.

Merged as of libsepol 1.9.41 and libsemanage 1.3.58.

handle is still present in def_role interface.  Not sure whether you
want it dropped from both sepol and semanage interfaces or just the
sepol interface, even though it is void in both (non-error reporting?).

-- 
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] Remove defrole from sepol

[Ivan Gyurdiev]

Stephen Smalley wrote:
> On Sat, 2005-11-19 at 00:50 -0500, Ivan Gyurdiev wrote:
>   
>> This patch removes defrole from sepol, because it does not belong there, 
>> and it's just plain wrong. The default role is not preserved in the 
>> binary policy - therefore it can only exist in semanage (unless we 
>> change the policy format to contain it). This simplifies user_record.c.
>>
>> It also updates del_role to have a void return type, as it can no longer 
>> fail.
>>
>> Now we need to add the labeling prefix back into semanage somehow.
>>     
>
> Merged as of libsepol 1.9.41 and libsemanage 1.3.58.
>
> handle is still present in def_role interface.  Not sure whether you
> want it dropped from both sepol and semanage interfaces or just the
> sepol interface, even though it is void in both (non-error reporting?).
>   
Yes, the handle should be removed... (at least from sepol).
The semanage interface should probably be removed too...



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