[PATCH] libsemanage/semanage - permission check for semanage

[PATCH] libsemanage/semanage - permission check for semanage

[Joshua Brindle]

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

- add semanage_can_write to libsemanage which does a silent check for
access on the active store, modules directory and binary policy
directory
- chance semanage_is_managed to use can_write instead of create_store
for access check
- add access check to seobject.py, in semanageRecord init
- remove dans UID == 0 check
- make the bottom level Makefile propagate install-pywrap target
- make install-pywrap target in libselinux depend on pywrap


It appears that the last commit didn't have make swigify run in
libsemanage so the generated swig wrappers were out of date, the ones in
this patch include my changes as well as the last ones.

Also, it seems that the current semanage is not disconnecting from the
store or freeing the handle. While this isn't too much of a problem
(aside from memory leaks) now, it will be in the future with the policy
server.

[-- Attachment #2: 1-semanage-write-check.diff --]
[-- Type: text/x-patch, Size: 13540 bytes --]

diff -purN -x.svn libselinux/src/Makefile libselinux/src/Makefile
--- libselinux/src/Makefile	2006-01-18 11:53:22.000000000 -0500
+++ libselinux/src/Makefile	2006-01-19 15:11:06.000000000 -0500
@@ -63,7 +63,7 @@ install: all 
 	install -m 755 $(LIBSO) $(SHLIBDIR)
 	cd $(LIBDIR) && ln -sf ../../`basename $(SHLIBDIR)`/$(LIBSO) $(TARGET)
 
-install-pywrap: 
+install-pywrap: pywrap
 	test -d $(PYTHONLIBDIR)/site-packages || install -m 755 -d $(PYTHONLIBDIR)/site-packages
 	install -m 755 $(SWIGFILES) $(PYTHONLIBDIR)/site-packages
 
diff -purN -x.svn libsemanage/include/semanage/handle.h libsemanage/include/semanage/handle.h
--- libsemanage/include/semanage/handle.h	2006-01-18 11:54:04.000000000 -0500
+++ libsemanage/include/semanage/handle.h	2006-01-19 16:33:23.000000000 -0500
@@ -100,6 +100,9 @@ int semanage_begin_transaction(semanage_
  */
 int semanage_commit(semanage_handle_t *);
 
+/* returns -1 if the store is probably not writable by the current UID/GID */
+int semanage_can_write(semanage_handle_t *sh);
+
 /* META NOTES
  *
  * For all functions a non-negative number indicates success. For some
diff -purN -x.svn libsemanage/src/direct_api.c libsemanage/src/direct_api.c
--- libsemanage/src/direct_api.c	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/direct_api.c	2006-01-19 15:11:05.000000000 -0500
@@ -82,7 +82,8 @@ int semanage_direct_is_managed(semanage_
 	if (semanage_check_init(polpath))
 		goto err;
 
-	if (semanage_create_store(sh, 0) < 0) 
+	/* manage test should be silent */
+	if (semanage_can_write(sh) < 0) 
 		return 0;
 
 	return 1;
@@ -775,3 +776,9 @@ static int semanage_direct_list(semanage
         }
 	return retval;
 }
+
+/* returns -1 if the store or binary policy directory 
+ * is probably not writable by the current UID/GID */
+int semanage_direct_can_write(semanage_handle_t *sh) {
+	return semanage_store_writable(sh);
+}
diff -purN -x.svn libsemanage/src/direct_api.h libsemanage/src/direct_api.h
--- libsemanage/src/direct_api.h	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/direct_api.h	2006-01-19 15:11:05.000000000 -0500
@@ -37,4 +37,6 @@ int semanage_direct_connect(
 int semanage_direct_is_managed(
 	struct semanage_handle *sh);
 
+int semanage_direct_can_write(struct semanage_handle *sh);
+
 #endif
diff -purN -x.svn libsemanage/src/handle.c libsemanage/src/handle.c
--- libsemanage/src/handle.c	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/handle.c	2006-01-19 16:33:33.000000000 -0500
@@ -142,6 +142,20 @@ int semanage_connect(semanage_handle_t *
 	return 0;
 }
 
+int semanage_can_write(semanage_handle_t *sh) {
+	assert(sh != NULL);
+	switch (sh->conf->store_type) {
+	case SEMANAGE_CON_DIRECT: 
+		return semanage_direct_can_write(sh);
+	default:
+		return -1;
+	}
+
+	return -1; /* unreachable */
+}
+
+hidden_def(semanage_can_write)
+
 int semanage_disconnect(semanage_handle_t *sh) {
 	assert(sh != NULL && sh->funcs != NULL && sh->funcs->disconnect != NULL);
 	if (!sh->is_connected) {
diff -purN -x.svn libsemanage/src/handle_internal.h libsemanage/src/handle_internal.h
--- libsemanage/src/handle_internal.h	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/handle_internal.h	2006-01-19 11:37:11.000000000 -0500
@@ -7,5 +7,6 @@
 hidden_proto(semanage_begin_transaction)
 hidden_proto(semanage_handle_destroy)
 hidden_proto(semanage_reload_policy)
+hidden_proto(semanage_can_write)
 
 #endif
diff -purN -x.svn libsemanage/src/libsemanage.map libsemanage/src/libsemanage.map
--- libsemanage/src/libsemanage.map	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/libsemanage.map	2006-01-19 11:37:24.000000000 -0500
@@ -11,6 +11,6 @@ LIBSEMANAGE_1.0 {
 	  semanage_reload_policy; semanage_set_reload; semanage_set_rebuild;
 	  semanage_user_*; semanage_bool_*; semanage_seuser_*;
 	  semanage_iface_*; semanage_port_*; semanage_context_*;
-	  semanage_fcontext_*;
+	  semanage_fcontext_*; semanage_can_write;
   local: *;
 };
diff -purN -x.svn libsemanage/src/semanage.py libsemanage/src/semanage.py
--- libsemanage/src/semanage.py	2006-01-19 15:45:19.000000000 -0500
+++ libsemanage/src/semanage.py	2006-01-19 15:43:18.000000000 -0500
@@ -88,6 +88,8 @@ semanage_begin_transaction = _semanage.s
 
 semanage_commit = _semanage.semanage_commit
 
+semanage_can_write = _semanage.semanage_can_write
+
 semanage_module_install = _semanage.semanage_module_install
 
 semanage_module_upgrade = _semanage.semanage_module_upgrade
@@ -258,6 +260,10 @@ semanage_user_get_name = _semanage.seman
 
 semanage_user_set_name = _semanage.semanage_user_set_name
 
+semanage_user_get_prefix = _semanage.semanage_user_get_prefix
+
+semanage_user_set_prefix = _semanage.semanage_user_set_prefix
+
 semanage_user_get_mlslevel = _semanage.semanage_user_get_mlslevel
 
 semanage_user_set_mlslevel = _semanage.semanage_user_set_mlslevel
diff -purN -x.svn libsemanage/src/semanage_store.c libsemanage/src/semanage_store.c
--- libsemanage/src/semanage_store.c	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/semanage_store.c	2006-01-19 16:36:53.000000000 -0500
@@ -281,7 +281,41 @@ int semanage_create_store(semanage_handl
 	return 0;
 }
 
+/* returns -1 if the store or binary policy directory 
+ * is probably not writable by the current UID/GID */
+int semanage_store_writable(semanage_handle_t *sh) {
+	const char *path;
+	char *path2, *path3, polpath[PATH_MAX];
+	int rc;
 
+	snprintf(polpath, PATH_MAX, "%s%s", selinux_path(), sh->conf->store_path);
+
+	if (semanage_check_init(polpath))
+		return -1;
+
+	/* check the active directory */
+	path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_TOPLEVEL);
+	if ((rc = access(path, R_OK | W_OK | X_OK)) != 0)
+		return rc;
+
+	/* check the binary policy install directory */
+	path = selinux_binary_policy_path();
+	path2 = strdup(path);
+	path3 = dirname(path2);
+
+	if ((rc = access(path3, R_OK | W_OK | X_OK)) != 0) {
+		free(path2);
+		return rc;
+	}
+	free(path2);
+
+	/* check the modules directory */
+	path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_MODULES);
+	if ((rc = access(path, R_OK | W_OK | X_OK)) != 0)
+		return rc;
+	
+	return 0;
+}
 
 /********************* other I/O functions *********************/
 
diff -purN -x.svn libsemanage/src/semanage_store.h libsemanage/src/semanage_store.h
--- libsemanage/src/semanage_store.h	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/semanage_store.h	2006-01-19 15:11:05.000000000 -0500
@@ -99,5 +99,6 @@ int semanage_verify_modules(
 int semanage_verify_linked(semanage_handle_t *sh);
 int semanage_verify_kernel(semanage_handle_t *sh);
 int semanage_split_fc(semanage_handle_t *sh);
+int semanage_store_writable(semanage_handle_t *sh);
 
 #endif
diff -purN -x.svn libsemanage/src/semanageswig_wrap.c libsemanage/src/semanageswig_wrap.c
--- libsemanage/src/semanageswig_wrap.c	2006-01-19 15:45:19.000000000 -0500
+++ libsemanage/src/semanageswig_wrap.c	2006-01-19 15:43:18.000000000 -0500
@@ -1748,6 +1748,8 @@ int semanage_user_compare(semanage_user_
 int semanage_user_compare2(semanage_user_t const *,semanage_user_t const *);
 char const *semanage_user_get_name(semanage_user_t const *);
 int semanage_user_set_name(semanage_handle_t *,semanage_user_t *,char const *);
+char const *semanage_user_get_prefix(semanage_user_t const *);
+int semanage_user_set_prefix(semanage_handle_t *,semanage_user_t *,char const *);
 char const *semanage_user_get_mlslevel(semanage_user_t const *);
 int semanage_user_set_mlslevel(semanage_handle_t *,semanage_user_t *,char const *);
 char const *semanage_user_get_mlsrange(semanage_user_t const *);
@@ -2396,6 +2398,26 @@ static PyObject *_wrap_semanage_commit(P
 }
 
 
+static PyObject *_wrap_semanage_can_write(PyObject *self, PyObject *args) {
+    PyObject *resultobj;
+    semanage_handle_t *arg1 = (semanage_handle_t *) 0 ;
+    int result;
+    PyObject * obj0 = 0 ;
+    
+    if(!PyArg_ParseTuple(args,(char *)"O:semanage_can_write",&obj0)) goto fail;
+    SWIG_Python_ConvertPtr(obj0, (void **)&arg1, SWIGTYPE_p_semanage_handle, SWIG_POINTER_EXCEPTION | 0);
+    if (SWIG_arg_fail(1)) SWIG_fail;
+    result = (int)semanage_can_write(arg1);
+    
+    {
+        resultobj = SWIG_From_int((int)(result)); 
+    }
+    return resultobj;
+    fail:
+    return NULL;
+}
+
+
 static PyObject *_wrap_semanage_module_install(PyObject *self, PyObject *args) {
     PyObject *resultobj;
     semanage_handle_t *arg1 = (semanage_handle_t *) 0 ;
@@ -4658,6 +4680,53 @@ static PyObject *_wrap_semanage_user_set
 }
 
 
+static PyObject *_wrap_semanage_user_get_prefix(PyObject *self, PyObject *args) {
+    PyObject *resultobj;
+    semanage_user_t *arg1 = (semanage_user_t *) 0 ;
+    char *result;
+    PyObject * obj0 = 0 ;
+    
+    if(!PyArg_ParseTuple(args,(char *)"O:semanage_user_get_prefix",&obj0)) goto fail;
+    SWIG_Python_ConvertPtr(obj0, (void **)&arg1, SWIGTYPE_p_semanage_user, SWIG_POINTER_EXCEPTION | 0);
+    if (SWIG_arg_fail(1)) SWIG_fail;
+    result = (char *)semanage_user_get_prefix((semanage_user_t const *)arg1);
+    
+    resultobj = SWIG_FromCharPtr(result);
+    return resultobj;
+    fail:
+    return NULL;
+}
+
+
+static PyObject *_wrap_semanage_user_set_prefix(PyObject *self, PyObject *args) {
+    PyObject *resultobj;
+    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 ;
+    
+    if(!PyArg_ParseTuple(args,(char *)"OOO:semanage_user_set_prefix",&obj0,&obj1,&obj2)) goto fail;
+    SWIG_Python_ConvertPtr(obj0, (void **)&arg1, SWIGTYPE_p_semanage_handle, SWIG_POINTER_EXCEPTION | 0);
+    if (SWIG_arg_fail(1)) SWIG_fail;
+    SWIG_Python_ConvertPtr(obj1, (void **)&arg2, SWIGTYPE_p_semanage_user, SWIG_POINTER_EXCEPTION | 0);
+    if (SWIG_arg_fail(2)) SWIG_fail;
+    if (!SWIG_AsCharPtr(obj2, (char**)&arg3)) {
+        SWIG_arg_fail(3);SWIG_fail;
+    }
+    result = (int)semanage_user_set_prefix(arg1,arg2,(char const *)arg3);
+    
+    {
+        resultobj = SWIG_From_int((int)(result)); 
+    }
+    return resultobj;
+    fail:
+    return NULL;
+}
+
+
 static PyObject *_wrap_semanage_user_get_mlslevel(PyObject *self, PyObject *args) {
     PyObject *resultobj;
     semanage_user_t *arg1 = (semanage_user_t *) 0 ;
@@ -7423,6 +7492,7 @@ static PyMethodDef SwigMethods[] = {
 	 { (char *)"semanage_disconnect", _wrap_semanage_disconnect, METH_VARARGS, NULL},
 	 { (char *)"semanage_begin_transaction", _wrap_semanage_begin_transaction, METH_VARARGS, NULL},
 	 { (char *)"semanage_commit", _wrap_semanage_commit, METH_VARARGS, NULL},
+	 { (char *)"semanage_can_write", _wrap_semanage_can_write, METH_VARARGS, NULL},
 	 { (char *)"semanage_module_install", _wrap_semanage_module_install, METH_VARARGS, NULL},
 	 { (char *)"semanage_module_upgrade", _wrap_semanage_module_upgrade, METH_VARARGS, NULL},
 	 { (char *)"semanage_module_install_base", _wrap_semanage_module_install_base, METH_VARARGS, NULL},
@@ -7508,6 +7578,8 @@ static PyMethodDef SwigMethods[] = {
 	 { (char *)"semanage_user_compare2", _wrap_semanage_user_compare2, METH_VARARGS, NULL},
 	 { (char *)"semanage_user_get_name", _wrap_semanage_user_get_name, METH_VARARGS, NULL},
 	 { (char *)"semanage_user_set_name", _wrap_semanage_user_set_name, METH_VARARGS, NULL},
+	 { (char *)"semanage_user_get_prefix", _wrap_semanage_user_get_prefix, METH_VARARGS, NULL},
+	 { (char *)"semanage_user_set_prefix", _wrap_semanage_user_set_prefix, METH_VARARGS, NULL},
 	 { (char *)"semanage_user_get_mlslevel", _wrap_semanage_user_get_mlslevel, METH_VARARGS, NULL},
 	 { (char *)"semanage_user_set_mlslevel", _wrap_semanage_user_set_mlslevel, METH_VARARGS, NULL},
 	 { (char *)"semanage_user_get_mlsrange", _wrap_semanage_user_get_mlsrange, METH_VARARGS, NULL},
diff -purN -x.svn Makefile Makefile
--- Makefile	2005-10-13 13:36:35.000000000 -0400
+++ Makefile	2006-01-19 15:46:30.000000000 -0500
@@ -1,4 +1,5 @@
 SUBDIRS=libsepol libselinux libsemanage checkpolicy policycoreutils # policy
+PYSUBDIRS=libselinux libsemanage
 
 ifeq ($(DEBUG),1)
 	export CFLAGS = -g3 -O0 -gdwarf-2 -fno-strict-aliasing -Wall -Wshadow
@@ -9,7 +10,11 @@ install relabel: 
 	@for subdir in $(SUBDIRS); do \
 		(cd $$subdir && $(MAKE) $@) || exit 1; \
 	done
-#	cd policy && make install-src
+
+install-pywrap:
+	@for subdir in $(PYSUBDIRS); do \
+		(cd $$subdir && $(MAKE) $@) || exit 1; \
+	done
 
 clean:
 	@for subdir in $(SUBDIRS); do \
diff -purN -x.svn policycoreutils/semanage/semanage policycoreutils/semanage/semanage
--- policycoreutils/semanage/semanage	2006-01-19 15:04:23.000000000 -0500
+++ policycoreutils/semanage/semanage	2006-01-19 15:11:06.000000000 -0500
@@ -24,9 +24,6 @@ import os, sys, getopt
 import seobject
 
 if __name__ == '__main__':
-	if os.getuid() > 0 or os.geteuid() > 0:
-		print "You must be root to run %s." % sys.argv[0]
-		sys.exit(0)
 
 	def usage(message = ""):
 		print '\
diff -purN -x.svn policycoreutils/semanage/seobject.py policycoreutils/semanage/seobject.py
--- policycoreutils/semanage/seobject.py	2006-01-19 15:04:23.000000000 -0500
+++ policycoreutils/semanage/seobject.py	2006-01-19 16:34:36.000000000 -0500
@@ -142,6 +142,11 @@ class setransRecords:
 class semanageRecords:
 	def __init__(self):
 		self.sh = semanage_handle_create()
+		rc = semanage_can_write(self.sh)
+		if rc:
+			semanage_handle_destroy(self.sh)
+			raise ValueError("Cannot write to policy directory.")
+
 		self.semanaged = semanage_is_managed(self.sh)
 		if self.semanaged:
 			semanage_connect(self.sh)

Re: [PATCH] libsemanage/semanage - permission check for semanage

[Ivan Gyurdiev]

> - chance semanage_is_managed to use can_write instead of create_store
> for access check
>   
Why should semanage_is_managed do an access check?
How does the lack of access mean that the store isn't managed?
I see in seobject, both is_managed, and can_write are called now, 
meaning two access checks
(and shouldn't they be in the opposite order).



--
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: [PATCH] libsemanage/semanage - permission check for semanage

[Joshua Brindle]

Ivan Gyurdiev wrote:
> 
>> - chance semanage_is_managed to use can_write instead of create_store
>> for access check
>>   
> 
> Why should semanage_is_managed do an access check?
the only way to check for a managed policy is to look for the 
directories. We also require write access to read from the store (read 
lock) so you can't do anything if you can't write to it. Also, it was 
already doing access checks (semanage_create_store with argument 0 
checks access but doesn't create directories) and that was what was 
spamming the user with errors when they couldn't write.

> How does the lack of access mean that the store isn't managed?

for all intents and purposes yes, since you can't query or write to it.

> I see in seobject, both is_managed, and can_write are called now, 
> meaning two access checks
> (and shouldn't they be in the opposite order).
> 

I'd like to bail as soon as possible if you aren't going to be able to 
write. Since they both do essentially the same checks but can_write is 
silent it should be first so that the user doesn't see the debug errors.

--
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: [PATCH] libsemanage/semanage - permission check for semanage

[Ivan Gyurdiev]

>> How does the lack of access mean that the store isn't managed?
>
> for all intents and purposes yes, since you can't query or write to it.
I still think that you shouldn't draw conclusions about what the user 
will be doing with the store in a function that answers the question "Is 
the store managed?" Maybe I'll switch to a more privileged user when I 
decide to query or write to the store.
>> I see in seobject, both is_managed, and can_write are called now, 
>> meaning two access checks
>> (and shouldn't they be in the opposite order).
>>
>
> I'd like to bail as soon as possible if you aren't going to be able to 
> write. Since they both do essentially the same checks but can_write is 
> silent it should be first so that the user doesn't see the debug errors.
This doesn't make sense to me, is_managed() should also be silent in the 
success path (meaning, if it can successfully check if the store is 
managed or not).

--
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: [PATCH] libsemanage/semanage - permission check for semanage

[Joshua Brindle]

Ivan Gyurdiev wrote:
> 
>>> How does the lack of access mean that the store isn't managed?
>>
>>
>> for all intents and purposes yes, since you can't query or write to it.
> 
> I still think that you shouldn't draw conclusions about what the user 
> will be doing with the store in a function that answers the question "Is 
> the store managed?" Maybe I'll switch to a more privileged user when I 
> decide to query or write to the store.
> 
I guess I can use access R_OK | X_OK to see if it's managed but 
ultimately the user won't be able to do anything if they can't write.

>>> I see in seobject, both is_managed, and can_write are called now, 
>>> meaning two access checks
>>> (and shouldn't they be in the opposite order).
>>>
>>
>> I'd like to bail as soon as possible if you aren't going to be able to 
>> write. Since they both do essentially the same checks but can_write is 
>> silent it should be first so that the user doesn't see the debug errors.
> 
> This doesn't make sense to me, is_managed() should also be silent in the 
> success path (meaning, if it can successfully check if the store is 
> managed or not).
> 
the reason Dan put in the UID == 0 check that I was trying to get rid of 
was because failure to read/write to the store spammed the user with 
error messages that don't make any sense to a user. Checking if they can 
  write before attempting to do so is all that makes sense, which is 
what I did. is_managed was always silent in the success path, it was not 
in the failure path which is why i switched it to use can_write instead 
of create_store, which has ERR() in all failure paths.

--
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: [PATCH] libsemanage/semanage - permission check for semanage

[Stephen Smalley]

On Thu, 2006-01-19 at 16:45 -0500, Joshua Brindle wrote:
> - add semanage_can_write to libsemanage which does a silent check for
> access on the active store, modules directory and binary policy
> directory
> - chance semanage_is_managed to use can_write instead of create_store
> for access check
> - add access check to seobject.py, in semanageRecord init
> - remove dans UID == 0 check
> - make the bottom level Makefile propagate install-pywrap target
> - make install-pywrap target in libselinux depend on pywrap
> 
> 
> It appears that the last commit didn't have make swigify run in
> libsemanage so the generated swig wrappers were out of date, the ones in
> this patch include my changes as well as the last ones.
> 
> Also, it seems that the current semanage is not disconnecting from the
> store or freeing the handle. While this isn't too much of a problem
> (aside from memory leaks) now, it will be in the future with the policy
> server.

diff -purN -x.svn libsemanage/include/semanage/handle.h libsemanage/include/semanage/handle.h
--- libsemanage/include/semanage/handle.h	2006-01-18 11:54:04.000000000 -0500
+++ libsemanage/include/semanage/handle.h	2006-01-19 16:33:23.000000000 -0500
@@ -100,6 +100,9 @@ int semanage_begin_transaction(semanage_
  */
 int semanage_commit(semanage_handle_t *);
 
+/* returns -1 if the store is probably not writable by the current UID/GID */
+int semanage_can_write(semanage_handle_t *sh);

The comment might be a little misleading, as:
a) access(2) checks against the real UID/GID of the process, not its
effective or fs identities; not presently an issue since the caller is
not a setuid or setgid program, but could be an issue at some point,
b) access(2) internally calls permission(9), and permission(9) calls the
security hook, so SELinux checks are also applied here.

diff -purN -x.svn libsemanage/src/semanage_store.c libsemanage/src/semanage_store.c
--- libsemanage/src/semanage_store.c	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/semanage_store.c	2006-01-19 16:36:53.000000000 -0500
@@ -281,7 +281,41 @@ int semanage_create_store(semanage_handl
 	return 0;
 }
 
+/* returns -1 if the store or binary policy directory 
+ * is probably not writable by the current UID/GID */
+int semanage_store_writable(semanage_handle_t *sh) {

Ditto.

+	const char *path;
+	char *path2, *path3, polpath[PATH_MAX];
+	int rc;
 
+	snprintf(polpath, PATH_MAX, "%s%s", selinux_path(), sh->conf->store_path);
+
+	if (semanage_check_init(polpath))
+		return -1;

These calls presently occur from semanage_direct* functions only, prior
to calling semanage_create_store.  Not sure why this would be different.
+
+	/* check the active directory */
+	path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_TOPLEVEL);
+	if ((rc = access(path, R_OK | W_OK | X_OK)) != 0)
+		return rc;
+
+	/* check the binary policy install directory */
+	path = selinux_binary_policy_path();
+	path2 = strdup(path);
+	path3 = dirname(path2);
+
+	if ((rc = access(path3, R_OK | W_OK | X_OK)) != 0) {
+		free(path2);
+		return rc;
+	}
+	free(path2);

Not sure about requiring rwx to the installed policy directory for this
test.  As far as DAC permissions are concerned, this ends up being
equivalent presently to checking rwx to the module store, as they are
both root owned and only writable by owner, but conceptually being able
to manipulate the store (particularly for read ops) differs from being
able to install a new policy.  SELinux policy might allow one but not
the other.

+
+	/* check the modules directory */
+	path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_MODULES);
+	if ((rc = access(path, R_OK | W_OK | X_OK)) != 0)
+		return rc;
+	
+	return 0;
+}

Do we need both checks (active toplevel and active modules), or will one do?
semanage_create_store() ends up doing multiple checks since it also
creates the directories along the way, but if we are just checking
access, it is likely sufficient to check the most restrictive one (i.e.
active modules), right?
 
 /********************* other I/O functions *********************/
 
diff -purN -x.svn policycoreutils/semanage/seobject.py policycoreutils/semanage/seobject.py
--- policycoreutils/semanage/seobject.py	2006-01-19 15:04:23.000000000 -0500
+++ policycoreutils/semanage/seobject.py	2006-01-19 16:34:36.000000000 -0500
@@ -142,6 +142,11 @@ class setransRecords:
 class semanageRecords:
 	def __init__(self):
 		self.sh = semanage_handle_create()
+		rc = semanage_can_write(self.sh)
+		if rc:
+			semanage_handle_destroy(self.sh)
+			raise ValueError("Cannot write to policy directory.")
+
 		self.semanaged = semanage_is_managed(self.sh)
 		if self.semanaged:
 			semanage_connect(self.sh)

Particularly given that this is called from __init__, it seems like the
check on the installed policy directory is too strong, as it could
interfere with module store read operations.

-- 
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: [PATCH] libsemanage/semanage - permission check for semanage

[Joshua Brindle]

Stephen Smalley wrote:
<snip>

> 
> The comment might be a little misleading, as:
> a) access(2) checks against the real UID/GID of the process, not its
> effective or fs identities; not presently an issue since the caller is
> not a setuid or setgid program, but could be an issue at some point,
> b) access(2) internally calls permission(9), and permission(9) calls the
> security hook, so SELinux checks are also applied here.
> 
Ok, that can be removed/updated. It is better IMO that the selinux 
checks apply since those will be just as much or more of an issue than 
uid/gid on a strict system

> diff -purN -x.svn libsemanage/src/semanage_store.c libsemanage/src/semanage_store.c
> --- libsemanage/src/semanage_store.c	2006-01-18 11:56:33.000000000 -0500
> +++ libsemanage/src/semanage_store.c	2006-01-19 16:36:53.000000000 -0500
> @@ -281,7 +281,41 @@ int semanage_create_store(semanage_handl
>  	return 0;
>  }
>  
> +/* returns -1 if the store or binary policy directory 
> + * is probably not writable by the current UID/GID */
> +int semanage_store_writable(semanage_handle_t *sh) {
> 
> Ditto.
> 
same

> +	const char *path;
> +	char *path2, *path3, polpath[PATH_MAX];
> +	int rc;
>  
> +	snprintf(polpath, PATH_MAX, "%s%s", selinux_path(), sh->conf->store_path);
> +
> +	if (semanage_check_init(polpath))
> +		return -1;
> 
> These calls presently occur from semanage_direct* functions only, prior
> to calling semanage_create_store.  Not sure why this would be different.

semanage_can_write can be called separately. See the seobject.py patch 
which calls it before anything that calls semanage_check_init. I suppose 
I can move it to the direct_can_write but it is an semanage_store 
function and this will only be called by direct_can_hook, not by ps or 
any other api

> +
> +	/* check the active directory */
> +	path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_TOPLEVEL);
> +	if ((rc = access(path, R_OK | W_OK | X_OK)) != 0)
> +		return rc;
> +
> +	/* check the binary policy install directory */
> +	path = selinux_binary_policy_path();
> +	path2 = strdup(path);
> +	path3 = dirname(path2);
> +
> +	if ((rc = access(path3, R_OK | W_OK | X_OK)) != 0) {
> +		free(path2);
> +		return rc;
> +	}
> +	free(path2);
> 
> Not sure about requiring rwx to the installed policy directory for this
> test.  As far as DAC permissions are concerned, this ends up being
> equivalent presently to checking rwx to the module store, as they are
> both root owned and only writable by owner, but conceptually being able
> to manipulate the store (particularly for read ops) differs from being
> able to install a new policy.  SELinux policy might allow one but not
> the other.
> 
hrm, at some point we need to test the writability of the binary policy 
dir. If the whole commit goes through and then the policy can't be 
written there is a consistency issue on the system


> +
> +	/* check the modules directory */
> +	path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_MODULES);
> +	if ((rc = access(path, R_OK | W_OK | X_OK)) != 0)
> +		return rc;
> +	
> +	return 0;
> +}
> 
> Do we need both checks (active toplevel and active modules), or will one do?
> semanage_create_store() ends up doing multiple checks since it also
> creates the directories along the way, but if we are just checking
> access, it is likely sufficient to check the most restrictive one (i.e.
> active modules), right?
>  
sure.

>  /********************* other I/O functions *********************/
>  
> diff -purN -x.svn policycoreutils/semanage/seobject.py policycoreutils/semanage/seobject.py
> --- policycoreutils/semanage/seobject.py	2006-01-19 15:04:23.000000000 -0500
> +++ policycoreutils/semanage/seobject.py	2006-01-19 16:34:36.000000000 -0500
> @@ -142,6 +142,11 @@ class setransRecords:
>  class semanageRecords:
>  	def __init__(self):
>  		self.sh = semanage_handle_create()
> +		rc = semanage_can_write(self.sh)
> +		if rc:
> +			semanage_handle_destroy(self.sh)
> +			raise ValueError("Cannot write to policy directory.")
> +
>  		self.semanaged = semanage_is_managed(self.sh)
>  		if self.semanaged:
>  			semanage_connect(self.sh)
> 
> Particularly given that this is called from __init__, it seems like the
> check on the installed policy directory is too strong, as it could
> interfere with module store read operations.
> 

fair enough, I'll rebase a patch today with these concerns addressed

--
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: [PATCH] libsemanage/semanage - permission check for semanage

[Stephen Smalley]

On Fri, 2006-01-20 at 09:00 -0500, Joshua Brindle wrote:
> semanage_can_write can be called separately. See the seobject.py patch 
> which calls it before anything that calls semanage_check_init. I suppose 
> I can move it to the direct_can_write but it is an semanage_store 
> function and this will only be called by direct_can_hook, not by ps or 
> any other api

Right, I was just looking for consistency in where check_init is called,
and presently it seems to always be done by the direct "layer" rather
than the store "layer".  

> hrm, at some point we need to test the writability of the binary policy 
> dir. If the whole commit goes through and then the policy can't be 
> written there is a consistency issue on the system

You already have rollback code, so the store should be rolled back to
the previous state which would already be consistent with the installed
policy.  You'd get two failure messages on both attempts to install the
binary policy file (first the new one, then the rolled back one), but
since it should already match the one from the previous sandbox, it
should be consistent nonetheless.

In any event, we shouldn't require write access to the policy directory
for all libsemanage operations.

-- 
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: [PATCH] libsemanage/semanage - permission check for semanage

[Stephen Smalley]

On Thu, 2006-01-19 at 16:45 -0500, Joshua Brindle wrote:
> - add semanage_can_write to libsemanage which does a silent check for
> access on the active store, modules directory and binary policy
> directory
> - chance semanage_is_managed to use can_write instead of create_store
> for access check
> - add access check to seobject.py, in semanageRecord init

Also, how do you envision implementing this interface for the policy
server backend?  Same question exists for semanage_is_managed I suppose,
but that interface seems a little more general in concept.
  
-- 
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: [PATCH] libsemanage/semanage - permission check for semanage

[Joshua Brindle]

Stephen Smalley wrote:
> On Thu, 2006-01-19 at 16:45 -0500, Joshua Brindle wrote:
> 
>>- add semanage_can_write to libsemanage which does a silent check for
>>access on the active store, modules directory and binary policy
>>directory
>>- chance semanage_is_managed to use can_write instead of create_store
>>for access check
>>- add access check to seobject.py, in semanageRecord init
> 
> 
> Also, how do you envision implementing this interface for the policy
> server backend?  Same question exists for semanage_is_managed I suppose,
> but that interface seems a little more general in concept.
>   

I wasn't at first but one could always ask the server if my context has 
write permission to the policy, which is a slightly different concept 
but from a user perspective probably means the same thing.

--
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: [PATCH] libsemanage/semanage - permission check for semanage

[Stephen Smalley]

On Thu, 2006-01-19 at 16:45 -0500, Joshua Brindle wrote:
> - make the bottom level Makefile propagate install-pywrap target
> - make install-pywrap target in libselinux depend on pywrap
> 
> 
> It appears that the last commit didn't have make swigify run in
> libsemanage so the generated swig wrappers were out of date, the ones in
> this patch include my changes as well as the last ones.

Merged the pywrap Makefile changes and regenerated the wrappers for the
prior changes.

-- 
Stephen Smalley
National Security Agency


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

Re: [PATCH] libsemanage/semanage - permission check for semanage

[Joshua Brindle]

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

On Fri, 2006-01-20 at 10:20 -0500, Stephen Smalley wrote:
> On Thu, 2006-01-19 at 16:45 -0500, Joshua Brindle wrote:
> > - make the bottom level Makefile propagate install-pywrap target
> > - make install-pywrap target in libselinux depend on pywrap
> > 
> > 
> > It appears that the last commit didn't have make swigify run in
> > libsemanage so the generated swig wrappers were out of date, the ones in
> > this patch include my changes as well as the last ones.
> 
> Merged the pywrap Makefile changes and regenerated the wrappers for the
> prior changes.
> 

attached patch addresses all of the concerns with the prior patch. The
main change is to make a function called semanage_access_check that
returns error, existance, readable or writable. Depending on where the
check is done test against one of them. 

I am now able to query the store as a non-root user after giving read
access to /modules and /modules/active{,/*} and read/write to
modules/semanage.read.LOCK

as expected I can query the store but not write to it, I also added a
check for writability at the beginning of begin_transaction so that the
user gets a nicer error when this happens.

[-- Attachment #2: 1-semanage-write-check.diff --]
[-- Type: text/x-patch, Size: 10699 bytes --]

diff -purN -x.svn libsemanage/include/semanage/handle.h libsemanage/include/semanage/handle.h
--- libsemanage/include/semanage/handle.h	2006-01-18 11:54:04.000000000 -0500
+++ libsemanage/include/semanage/handle.h	2006-01-20 09:43:33.000000000 -0500
@@ -100,6 +100,12 @@ int semanage_begin_transaction(semanage_
  */
 int semanage_commit(semanage_handle_t *);
 
+#define SEMANAGE_CAN_READ 1
+#define SEMANAGE_CAN_WRITE 2
+/* returns SEMANAGE_CAN_READ or SEMANAGE_CAN_WRITE if the store is readable
+ * or writable, respectively. <0 if an error occured */ 
+int semanage_access_check(semanage_handle_t *sh);
+
 /* META NOTES
  *
  * For all functions a non-negative number indicates success. For some
diff -purN -x.svn libsemanage/src/direct_api.c libsemanage/src/direct_api.c
--- libsemanage/src/direct_api.c	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/direct_api.c	2006-01-20 11:34:14.000000000 -0500
@@ -82,7 +82,7 @@ int semanage_direct_is_managed(semanage_
 	if (semanage_check_init(polpath))
 		goto err;
 
-	if (semanage_create_store(sh, 0) < 0) 
+	if (semanage_access_check(sh) < 0) 
 		return 0;
 
 	return 1;
@@ -102,7 +102,7 @@ int semanage_direct_connect(semanage_han
 	if (semanage_check_init(polpath))
 		goto err;
 
-	if (semanage_create_store(sh, 1) < 0) 
+	if (semanage_access_check(sh) < SEMANAGE_CAN_READ) 
 		goto err;
 
 	sh->u.direct.translock_file_fd = -1;
@@ -226,6 +226,10 @@ static int semanage_direct_disconnect(se
 }
 
 static int semanage_direct_begintrans(semanage_handle_t *sh) {
+	
+	if (semanage_access_check(sh) != SEMANAGE_CAN_WRITE) {
+		return -1;
+	}
 	if (semanage_get_trans_lock(sh) < 0) {
 		return -1;
 	}
@@ -775,3 +779,14 @@ static int semanage_direct_list(semanage
         }
 	return retval;
 }
+
+int semanage_direct_access_check(semanage_handle_t *sh) {
+	char polpath[PATH_MAX];	
+
+	snprintf(polpath, PATH_MAX, "%s%s", selinux_path(), sh->conf->store_path);
+
+	if (semanage_check_init(polpath))
+		return -1;
+
+	return semanage_store_access_check(sh);
+}
diff -purN -x.svn libsemanage/src/direct_api.h libsemanage/src/direct_api.h
--- libsemanage/src/direct_api.h	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/direct_api.h	2006-01-20 09:29:06.000000000 -0500
@@ -37,4 +37,6 @@ int semanage_direct_connect(
 int semanage_direct_is_managed(
 	struct semanage_handle *sh);
 
+int semanage_direct_access_check(struct semanage_handle *sh);
+
 #endif
diff -purN -x.svn libsemanage/src/handle.c libsemanage/src/handle.c
--- libsemanage/src/handle.c	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/handle.c	2006-01-20 09:26:57.000000000 -0500
@@ -142,6 +142,19 @@ int semanage_connect(semanage_handle_t *
 	return 0;
 }
 
+int semanage_access_check(semanage_handle_t *sh) {
+	assert(sh != NULL);
+	switch (sh->conf->store_type) {
+	case SEMANAGE_CON_DIRECT: 
+		return semanage_direct_access_check(sh);
+	default:
+		return -1;
+	}
+
+	return -1; /* unreachable */
+}
+hidden_def(semanage_access_check)
+
 int semanage_disconnect(semanage_handle_t *sh) {
 	assert(sh != NULL && sh->funcs != NULL && sh->funcs->disconnect != NULL);
 	if (!sh->is_connected) {
diff -purN -x.svn libsemanage/src/handle_internal.h libsemanage/src/handle_internal.h
--- libsemanage/src/handle_internal.h	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/handle_internal.h	2006-01-20 09:29:19.000000000 -0500
@@ -7,5 +7,6 @@
 hidden_proto(semanage_begin_transaction)
 hidden_proto(semanage_handle_destroy)
 hidden_proto(semanage_reload_policy)
+hidden_proto(semanage_access_check)
 
 #endif
diff -purN -x.svn libsemanage/src/libsemanage.map libsemanage/src/libsemanage.map
--- libsemanage/src/libsemanage.map	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/libsemanage.map	2006-01-20 09:27:13.000000000 -0500
@@ -11,6 +11,6 @@ LIBSEMANAGE_1.0 {
 	  semanage_reload_policy; semanage_set_reload; semanage_set_rebuild;
 	  semanage_user_*; semanage_bool_*; semanage_seuser_*;
 	  semanage_iface_*; semanage_port_*; semanage_context_*;
-	  semanage_fcontext_*;
+	  semanage_fcontext_*; semanage_access_check;
   local: *;
 };
diff -purN -x.svn libsemanage/src/semanage.py libsemanage/src/semanage.py
--- libsemanage/src/semanage.py	2006-01-20 11:30:59.000000000 -0500
+++ libsemanage/src/semanage.py	2006-01-20 11:42:19.000000000 -0500
@@ -87,6 +87,10 @@ semanage_disconnect = _semanage.semanage
 semanage_begin_transaction = _semanage.semanage_begin_transaction
 
 semanage_commit = _semanage.semanage_commit
+SEMANAGE_CAN_READ = _semanage.SEMANAGE_CAN_READ
+SEMANAGE_CAN_WRITE = _semanage.SEMANAGE_CAN_WRITE
+
+semanage_access_check = _semanage.semanage_access_check
 
 semanage_module_install = _semanage.semanage_module_install
 
diff -purN -x.svn libsemanage/src/semanage_store.c libsemanage/src/semanage_store.c
--- libsemanage/src/semanage_store.c	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/semanage_store.c	2006-01-20 11:28:36.000000000 -0500
@@ -281,7 +281,54 @@ int semanage_create_store(semanage_handl
 	return 0;
 }
 
+/* returns <0 if the active store cannot be read or doesn't exist
+ * 0 if the store exists but the lock file cannot be written to
+ * SEMANAGE_CAN_READ if the store can be read and the lock file written to
+ * SEMANAGE_CAN_WRITE if the modules directory and binary policy dir can be written to
+ */
+int semanage_store_access_check(semanage_handle_t *sh) {
+	const char *path;
+	char *path2, *path3;
+	int rc = -1;
+
+	/* read access on active store */
+	path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_TOPLEVEL);
+	if (access(path, R_OK | X_OK) != 0)
+		goto out;
+
+	/* we can read the active store meaning it is managed
+	 * so now we return 0 to indicate no error */
+	rc = 0;
+
+	/* read/write access on lock file required for reading */
+	path = semanage_files[SEMANAGE_READ_LOCK];
+	if (access(path, R_OK | W_OK) != 0)
+		goto out;
+
+	/* everything needed for reading has been checked */
+	rc = SEMANAGE_CAN_READ;
+
+	/* check the modules directory */
+	path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_MODULES);
+	if (access(path, R_OK | W_OK | X_OK) != 0)
+		goto out;
+
+	/* check the binary policy install directory */
+	path = selinux_binary_policy_path();
+	path2 = strdup(path);
+	path3 = dirname(path2);
+
+	if ((access(path3, R_OK | W_OK | X_OK)) != 0) {
+		free(path2);
+		goto out;
+	}
+	free(path2);
+	
+	rc = SEMANAGE_CAN_WRITE;
 
+out:
+	return rc;
+}
 
 /********************* other I/O functions *********************/
 
diff -purN -x.svn libsemanage/src/semanage_store.h libsemanage/src/semanage_store.h
--- libsemanage/src/semanage_store.h	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/semanage_store.h	2006-01-19 15:11:05.000000000 -0500
@@ -99,5 +99,6 @@ int semanage_verify_modules(
 int semanage_verify_linked(semanage_handle_t *sh);
 int semanage_verify_kernel(semanage_handle_t *sh);
 int semanage_split_fc(semanage_handle_t *sh);
+int semanage_store_writable(semanage_handle_t *sh);
 
 #endif
diff -purN -x.svn libsemanage/src/semanageswig_wrap.c libsemanage/src/semanageswig_wrap.c
--- libsemanage/src/semanageswig_wrap.c	2006-01-20 11:30:59.000000000 -0500
+++ libsemanage/src/semanageswig_wrap.c	2006-01-20 11:42:19.000000000 -0500
@@ -2398,6 +2398,26 @@ static PyObject *_wrap_semanage_commit(P
 }
 
 
+static PyObject *_wrap_semanage_access_check(PyObject *self, PyObject *args) {
+    PyObject *resultobj;
+    semanage_handle_t *arg1 = (semanage_handle_t *) 0 ;
+    int result;
+    PyObject * obj0 = 0 ;
+    
+    if(!PyArg_ParseTuple(args,(char *)"O:semanage_access_check",&obj0)) goto fail;
+    SWIG_Python_ConvertPtr(obj0, (void **)&arg1, SWIGTYPE_p_semanage_handle, SWIG_POINTER_EXCEPTION | 0);
+    if (SWIG_arg_fail(1)) SWIG_fail;
+    result = (int)semanage_access_check(arg1);
+    
+    {
+        resultobj = SWIG_From_int((int)(result)); 
+    }
+    return resultobj;
+    fail:
+    return NULL;
+}
+
+
 static PyObject *_wrap_semanage_module_install(PyObject *self, PyObject *args) {
     PyObject *resultobj;
     semanage_handle_t *arg1 = (semanage_handle_t *) 0 ;
@@ -7472,6 +7492,7 @@ static PyMethodDef SwigMethods[] = {
 	 { (char *)"semanage_disconnect", _wrap_semanage_disconnect, METH_VARARGS, NULL},
 	 { (char *)"semanage_begin_transaction", _wrap_semanage_begin_transaction, METH_VARARGS, NULL},
 	 { (char *)"semanage_commit", _wrap_semanage_commit, METH_VARARGS, NULL},
+	 { (char *)"semanage_access_check", _wrap_semanage_access_check, METH_VARARGS, NULL},
 	 { (char *)"semanage_module_install", _wrap_semanage_module_install, METH_VARARGS, NULL},
 	 { (char *)"semanage_module_upgrade", _wrap_semanage_module_upgrade, METH_VARARGS, NULL},
 	 { (char *)"semanage_module_install_base", _wrap_semanage_module_install_base, METH_VARARGS, NULL},
@@ -8139,6 +8160,12 @@ SWIGEXPORT(void) SWIG_init(void) {
         PyDict_SetItemString(d,"SEMANAGE_CON_POLSERV_REMOTE", SWIG_From_int((int)(SEMANAGE_CON_POLSERV_REMOTE))); 
     }
     {
+        PyDict_SetItemString(d,"SEMANAGE_CAN_READ", SWIG_From_int((int)(1))); 
+    }
+    {
+        PyDict_SetItemString(d,"SEMANAGE_CAN_WRITE", SWIG_From_int((int)(2))); 
+    }
+    {
         PyDict_SetItemString(d,"SEMANAGE_PROTO_UDP", SWIG_From_int((int)(0))); 
     }
     {
diff -purN -x.svn policycoreutils/semanage/semanage policycoreutils/semanage/semanage
--- policycoreutils/semanage/semanage	2006-01-20 11:31:07.000000000 -0500
+++ policycoreutils/semanage/semanage	2006-01-20 11:37:33.000000000 -0500
@@ -24,9 +24,6 @@ import os, sys, getopt
 import seobject
 
 if __name__ == '__main__':
-	if os.getuid() > 0 or os.geteuid() > 0:
-		print "You must be root to run %s." % sys.argv[0]
-		sys.exit(0)
 
 	def usage(message = ""):
 		print '\
diff -purN -x.svn policycoreutils/semanage/seobject.py policycoreutils/semanage/seobject.py
--- policycoreutils/semanage/seobject.py	2006-01-20 11:31:07.000000000 -0500
+++ policycoreutils/semanage/seobject.py	2006-01-20 11:47:39.000000000 -0500
@@ -143,10 +143,19 @@ class semanageRecords:
 	def __init__(self):
 		self.sh = semanage_handle_create()
 		self.semanaged = semanage_is_managed(self.sh)
-		if self.semanaged:
-			rc = semanage_connect(self.sh)
-			if rc < 0:
-				raise ValueError("Could not establish semanage connection")
+		if not self.semanaged:
+			semanage_handle_destroy(self.sh)
+			raise ValueError("SELinux policy is not managed.")
+
+		rc = semanage_access_check(self.sh)
+		if rc < SEMANAGE_CAN_READ:
+			semanage_handle_destroy(self.sh)
+			raise ValueError("Cannot read policy store.")
+
+		rc = semanage_connect(self.sh)
+		if rc < 0:
+			semanage_handle_destroy(self.sh)
+			raise ValueError("Could not establish semanage connection")
 
 class loginRecords(semanageRecords):
 	def __init__(self):

Re: [PATCH] libsemanage/semanage - permission check for semanage

[Stephen Smalley]

On Fri, 2006-01-20 at 14:14 -0500, Joshua Brindle wrote:
> attached patch addresses all of the concerns with the prior patch. The
> main change is to make a function called semanage_access_check that
> returns error, existance, readable or writable. Depending on where the
> check is done test against one of them. 
> 
> I am now able to query the store as a non-root user after giving read
> access to /modules and /modules/active{,/*} and read/write to
> modules/semanage.read.LOCK
> 
> as expected I can query the store but not write to it, I also added a
> check for writability at the beginning of begin_transaction so that the
> user gets a nicer error when this happens.

Better, but a few lingering issues/questions:
1) Running semanage as a non-root user with existing default modes on
the policy module tree yields "SELinux policy is not managed."
So we aren't quite providing the right information to the user yet.

2) I'm still not sure about the check on the binary policy install
directory; not all "write" operations truly require rebuilding the
kernel binary policy, and we already have rollback code if that
installation fails, so I suspect that the writability check on the
active module store is good enough here.

3) I normally would have expected checking write access to the container
directory for the lock file rather than checking access to the lock file
itself, but I suppose since you create the lock file on the first
transaction and never remove it and use lockf for locking, it should
always exist if the store itself has been initialized?

-- 
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: [PATCH] libsemanage/semanage - permission check for semanage

[Joshua Brindle]

Stephen Smalley wrote:
> On Fri, 2006-01-20 at 14:14 -0500, Joshua Brindle wrote:
> 
>>attached patch addresses all of the concerns with the prior patch. The
>>main change is to make a function called semanage_access_check that
>>returns error, existance, readable or writable. Depending on where the
>>check is done test against one of them. 
>>
>>I am now able to query the store as a non-root user after giving read
>>access to /modules and /modules/active{,/*} and read/write to
>>modules/semanage.read.LOCK
>>
>>as expected I can query the store but not write to it, I also added a
>>check for writability at the beginning of begin_transaction so that the
>>user gets a nicer error when this happens.
> 
> 
> Better, but a few lingering issues/questions:
> 1) Running semanage as a non-root user with existing default modes on
> the policy module tree yields "SELinux policy is not managed."
> So we aren't quite providing the right information to the user yet.
>
if the user can't stat the module directory how do we know whether or 
not it is a managed system?


> 2) I'm still not sure about the check on the binary policy install
> directory; not all "write" operations truly require rebuilding the
> kernel binary policy, and we already have rollback code if that
> installation fails, so I suspect that the writability check on the
> active module store is good enough here.
> 
Only seuser doesn't require a policy rebuild, though I guess you could 
build in alternate stores without installing..

I guess that can be removed.

> 3) I normally would have expected checking write access to the container
> directory for the lock file rather than checking access to the lock file
> itself, but I suppose since you create the lock file on the first
> transaction and never remove it and use lockf for locking, it should
> always exist if the store itself has been initialized?
> 
the lock files are generally persistent, the current locking mechanism 
uses lockf instead of just testing for existence since lockf will 
release the lock on application exit.

I'd rather not give read only clients access to write to the module 
directory, even though it shouldn't matter as long as the active and 
previous directories are protected.

As far as querying goes read/write on the lock file is the absolute 
minimum required, so if that works they'll be able to query the store, 
which is what the access check should be checking. The only permission 
really needed to query the store could be lock but since the open call 
tries to create it if it doesn't exist, and also truncate then we have 
to allow read and write (also access doesn't have the ability to just 
test for lock)

--
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: [PATCH] libsemanage/semanage - permission check for semanage

[Stephen Smalley]

On Fri, 2006-01-20 at 16:25 -0500, Joshua Brindle wrote:
> Stephen Smalley wrote:
> > Better, but a few lingering issues/questions:
> > 1) Running semanage as a non-root user with existing default modes on
> > the policy module tree yields "SELinux policy is not managed."
> > So we aren't quite providing the right information to the user yet.
> >
> if the user can't stat the module directory how do we know whether or 
> not it is a managed system?

Understood, but the message to the user needs to convey that fact, i.e.
instead of just saying "SELinux policy is not managed", we likely need
to say "SELinux policy is not managed or the module store is not
accessible to you."  

We also need to make sure that setsebool continues to do the right thing
here, as it uses semanage_is_managed() to decide whether it should a)
abort with an error (if < 0), b) fall back to the libselinux support for
setting booleans (== 0), or proceed to use the semanage functions for
setting booleans (> 0).  It looks like this patch is fine in that
respect since it doesn't change that interface; it should just silence
the warning we get from libsemanage in (b).  

Trying setsebool as a normal user I get "Could not change active
booleans:  Permission denied", which seems like a reasonable error
message for that case.  Only potential concern is that what is really
happening there is that setsebool is falling back to case (b) and trying
to directly manipulate booleans via libselinux, and it is only the lack
of permission to do so that prevents it from proceeding.  So if the
caller lacked permission to access the module store but had permission
to directly manipulate booleans (a pathological situation indeed), then
setsebool could do the wrong thing here.

-- 
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: [PATCH] libsemanage/semanage - permission check for semanage

[Joshua Brindle]

Stephen Smalley wrote:
> On Fri, 2006-01-20 at 16:25 -0500, Joshua Brindle wrote:
> 
>>Stephen Smalley wrote:
>>
>>>Better, but a few lingering issues/questions:
>>>1) Running semanage as a non-root user with existing default modes on
>>>the policy module tree yields "SELinux policy is not managed."
>>>So we aren't quite providing the right information to the user yet.
>>>
>>
>>if the user can't stat the module directory how do we know whether or 
>>not it is a managed system?
> 
> 
> Understood, but the message to the user needs to convey that fact, i.e.
> instead of just saying "SELinux policy is not managed", we likely need
> to say "SELinux policy is not managed or the module store is not
> accessible to you."  

sounds good
> 
> We also need to make sure that setsebool continues to do the right thing
> here, as it uses semanage_is_managed() to decide whether it should a)
> abort with an error (if < 0), b) fall back to the libselinux support for
> setting booleans (== 0), or proceed to use the semanage functions for
> setting booleans (> 0).  It looks like this patch is fine in that
> respect since it doesn't change that interface; it should just silence
> the warning we get from libsemanage in (b).  
> 
yes, looking at setsebool it seems correct wrt is_managed. Which warning 
are you speaking of? In is_managed == 0 it uses 
selinux_set_boolean_list() and on success goes to out which destroys the 
handle and returns.

> Trying setsebool as a normal user I get "Could not change active
> booleans:  Permission denied", which seems like a reasonable error
> message for that case.  Only potential concern is that what is really
> happening there is that setsebool is falling back to case (b) and trying
> to directly manipulate booleans via libselinux, and it is only the lack
> of permission to do so that prevents it from proceeding.  So if the
> caller lacked permission to access the module store but had permission
> to directly manipulate booleans (a pathological situation indeed), then
> setsebool could do the wrong thing here.
> 

Hrm.. an interesting problem. How can we determine when the fallback 
should happen? It seems difficult to differenciate the 'not managed' and 
'no permission to see store' cases, which means setsebool can't know 
when to fallback and when not to. I'm not sure there is a solution to 
this problem except to encapsulate access to booleans through 
libsemanage clients, even in the targeted policy, which might not be 
acceptable.

--
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: [PATCH] libsemanage/semanage - permission check for semanage

[Stephen Smalley]

On Mon, 2006-01-23 at 09:51 -0500, Joshua Brindle wrote:
> yes, looking at setsebool it seems correct wrt is_managed. Which warning 
> are you speaking of? In is_managed == 0 it uses 
> selinux_set_boolean_list() and on success goes to out which destroys the 
> handle and returns.

I just mean the old libsemanage warning from the old
is_managed()->create_store() code path, which is eliminated by your
patch.  That is a good thing, not a problem.

> Hrm.. an interesting problem. How can we determine when the fallback 
> should happen? It seems difficult to differenciate the 'not managed' and 
> 'no permission to see store' cases, which means setsebool can't know 
> when to fallback and when not to. I'm not sure there is a solution to 
> this problem except to encapsulate access to booleans through 
> libsemanage clients, even in the targeted policy, which might not be 
> acceptable.

I suspect it isn't a real issue in practice, but if we thought it was
truly crucial, we could introduce an explicit definition of is_managed
rather than inferring it from active module store existence, whether via
a definition in /etc/selinux/config or some other indicator (e.g. an
enable/disable flag in semanage.conf analogous to the one for
setrans.conf).

-- 
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: [PATCH] libsemanage/semanage - permission check for semanage

[Joshua Brindle]

Stephen Smalley wrote:
> On Mon, 2006-01-23 at 09:51 -0500, Joshua Brindle wrote:
> 
>>yes, looking at setsebool it seems correct wrt is_managed. Which warning 
>>are you speaking of? In is_managed == 0 it uses 
>>selinux_set_boolean_list() and on success goes to out which destroys the 
>>handle and returns.
> 
> 
> I just mean the old libsemanage warning from the old
> is_managed()->create_store() code path, which is eliminated by your
> patch.  That is a good thing, not a problem.
> 
speaking of the create_store code path, That function is no longer in 
use and it was primarily used for bootstrapping non-managed systems so I 
didn't remove it yet but we can probably just require the package 
manager or make scripts to initialize the store and remove this from the 
library entirely.

> 
>>Hrm.. an interesting problem. How can we determine when the fallback 
>>should happen? It seems difficult to differenciate the 'not managed' and 
>>'no permission to see store' cases, which means setsebool can't know 
>>when to fallback and when not to. I'm not sure there is a solution to 
>>this problem except to encapsulate access to booleans through 
>>libsemanage clients, even in the targeted policy, which might not be 
>>acceptable.
> 
> 
> I suspect it isn't a real issue in practice, but if we thought it was
> truly crucial, we could introduce an explicit definition of is_managed
> rather than inferring it from active module store existence, whether via
> a definition in /etc/selinux/config or some other indicator (e.g. an
> enable/disable flag in semanage.conf analogous to the one for
> setrans.conf).
> 

I'd rather punt on that for now, in the case that it really matters 
(policy server) the store and boolean access will all be encapsulated 
and accessible only by the server.

Aside from the error string changes and the policy directory permission 
check removal does anything else need to be changed before resubmission? 
Should I go ahead and remove create_store?


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

Re: [PATCH] libsemanage/semanage - permission check for semanage

[Stephen Smalley]

On Mon, 2006-01-23 at 10:40 -0500, Joshua Brindle wrote:
> speaking of the create_store code path, That function is no longer in 
> use and it was primarily used for bootstrapping non-managed systems so I 
> didn't remove it yet but we can probably just require the package 
> manager or make scripts to initialize the store and remove this from the 
> library entirely.

Hmm...I missed that side effect.  Moving aside my module store and
trying to re-install the base module from /usr/share/selinux does indeed
fail against the new libsemanage (upon connect).  So the policy package
would have to pre-create the directory tree and the lock files so that
the connect could succeed?  I'm not sure that is truly what we want.

-- 
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: [PATCH] libsemanage/semanage - permission check for semanage

[Joshua Brindle]

Stephen Smalley wrote:
> On Mon, 2006-01-23 at 10:40 -0500, Joshua Brindle wrote:
> 
>>speaking of the create_store code path, That function is no longer in 
>>use and it was primarily used for bootstrapping non-managed systems so I 
>>didn't remove it yet but we can probably just require the package 
>>manager or make scripts to initialize the store and remove this from the 
>>library entirely.
> 
> 
> Hmm...I missed that side effect.  Moving aside my module store and
> trying to re-install the base module from /usr/share/selinux does indeed
> fail against the new libsemanage (upon connect).  So the policy package
> would have to pre-create the directory tree and the lock files so that
> the connect could succeed?  I'm not sure that is truly what we want.
> 

before it was implicitly happening if it didn't exist (which will now 
result in an 'unmanaged system' return which should tell the client not 
call connect and therefore not initialize the store (i love bootstrap 
issues btw). I'm alright with making an explicit option to initialize it 
via semanage or semodule (preferably semodule).. would this be better?


--
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: [PATCH] libsemanage/semanage - permission check for semanage

[Stephen Smalley]

On Mon, 2006-01-23 at 11:05 -0500, Joshua Brindle wrote:
> Stephen Smalley wrote:
> > On Mon, 2006-01-23 at 10:40 -0500, Joshua Brindle wrote:
> > 
> >>speaking of the create_store code path, That function is no longer in 
> >>use and it was primarily used for bootstrapping non-managed systems so I 
> >>didn't remove it yet but we can probably just require the package 
> >>manager or make scripts to initialize the store and remove this from the 
> >>library entirely.
> > 
> > 
> > Hmm...I missed that side effect.  Moving aside my module store and
> > trying to re-install the base module from /usr/share/selinux does indeed
> > fail against the new libsemanage (upon connect).  So the policy package
> > would have to pre-create the directory tree and the lock files so that
> > the connect could succeed?  I'm not sure that is truly what we want.
> > 
> 
> before it was implicitly happening if it didn't exist (which will now 
> result in an 'unmanaged system' return which should tell the client not 
> call connect and therefore not initialize the store (i love bootstrap 
> issues btw). I'm alright with making an explicit option to initialize it 
> via semanage or semodule (preferably semodule).. would this be better?

Why do we want changed behavior upon _connect at all?  Why not leave
connect alone (i.e. still call create_store(...,1) there), and just
change clients as desired to call access_check() when you don't want
such bootstrapping (which it seems is desirable as the default for
semodule -b)?

-- 
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: [PATCH] libsemanage/semanage - permission check for semanage

[Joshua Brindle]

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

On Mon, 2006-01-23 at 11:18 -0500, Stephen Smalley wrote:
> On Mon, 2006-01-23 at 11:05 -0500, Joshua Brindle wrote:
> > Stephen Smalley wrote:
> > > On Mon, 2006-01-23 at 10:40 -0500, Joshua Brindle wrote:
> > > 
> > >>speaking of the create_store code path, That function is no longer in 
> > >>use and it was primarily used for bootstrapping non-managed systems so I 
> > >>didn't remove it yet but we can probably just require the package 
> > >>manager or make scripts to initialize the store and remove this from the 
> > >>library entirely.
> > > 
> > > 
> > > Hmm...I missed that side effect.  Moving aside my module store and
> > > trying to re-install the base module from /usr/share/selinux does indeed
> > > fail against the new libsemanage (upon connect).  So the policy package
> > > would have to pre-create the directory tree and the lock files so that
> > > the connect could succeed?  I'm not sure that is truly what we want.
> > > 
> > 
> > before it was implicitly happening if it didn't exist (which will now 
> > result in an 'unmanaged system' return which should tell the client not 
> > call connect and therefore not initialize the store (i love bootstrap 
> > issues btw). I'm alright with making an explicit option to initialize it 
> > via semanage or semodule (preferably semodule).. would this be better?
> 
> Why do we want changed behavior upon _connect at all?  Why not leave
> connect alone (i.e. still call create_store(...,1) there), and just
> change clients as desired to call access_check() when you don't want
> such bootstrapping (which it seems is desirable as the default for
> semodule -b)?

This patch adds semanage_set_create_store which is callable by the
client to create the store. Set to 1 by semodule when called with -b.
Otherwise the same access checks as before are done.

Also adds semanage_is_connected to avoid an assertion failure in
semodule if an error occurs before connecting.

[-- Attachment #2: 1-semanage-write-check.diff --]
[-- Type: text/x-patch, Size: 16008 bytes --]

diff -x.svn -pruN libsemanage/include/semanage/handle.h libsemanage/include/semanage/handle.h
--- libsemanage/include/semanage/handle.h	2006-01-18 11:54:04.000000000 -0500
+++ libsemanage/include/semanage/handle.h	2006-01-26 15:35:51.000000000 -0500
@@ -64,6 +64,11 @@ void semanage_set_reload(semanage_handle
  * 1 for yes, 0 for no (default) */
 void semanage_set_rebuild(semanage_handle_t *handle, int do_rebuild);
 
+/* create the store if it does not exist, this only has an effect on 
+ * direct connections and must be called before semanage_connect 
+ * 1 for yes, 0 for no (default) */
+void semanage_set_create_store(semanage_handle_t *handle, int create_store);
+
 /* Check whether policy is managed via libsemanage on this system.
  * Must be called prior to trying to connect.
  * Return 1 if policy is managed via libsemanage on this system,
@@ -100,6 +105,15 @@ int semanage_begin_transaction(semanage_
  */
 int semanage_commit(semanage_handle_t *);
 
+#define SEMANAGE_CAN_READ 1
+#define SEMANAGE_CAN_WRITE 2
+/* returns SEMANAGE_CAN_READ or SEMANAGE_CAN_WRITE if the store is readable
+ * or writable, respectively. <0 if an error occured */ 
+int semanage_access_check(semanage_handle_t *sh);
+
+/* returns 0 if not connected, 1 if connected */
+int semanage_is_connected(semanage_handle_t *sh);
+
 /* META NOTES
  *
  * For all functions a non-negative number indicates success. For some
diff -x.svn -pruN libsemanage/src/direct_api.c libsemanage/src/direct_api.c
--- libsemanage/src/direct_api.c	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/direct_api.c	2006-01-26 14:55:44.000000000 -0500
@@ -82,7 +82,7 @@ int semanage_direct_is_managed(semanage_
 	if (semanage_check_init(polpath))
 		goto err;
 
-	if (semanage_create_store(sh, 0) < 0) 
+	if (semanage_access_check(sh) < 0) 
 		return 0;
 
 	return 1;
@@ -102,7 +102,11 @@ int semanage_direct_connect(semanage_han
 	if (semanage_check_init(polpath))
 		goto err;
 
-	if (semanage_create_store(sh, 1) < 0) 
+	if (sh->create_store)
+		if (semanage_create_store(sh, 1))
+			goto err;
+
+	if (semanage_access_check(sh) < SEMANAGE_CAN_READ) 
 		goto err;
 
 	sh->u.direct.translock_file_fd = -1;
@@ -226,6 +230,10 @@ static int semanage_direct_disconnect(se
 }
 
 static int semanage_direct_begintrans(semanage_handle_t *sh) {
+	
+	if (semanage_access_check(sh) != SEMANAGE_CAN_WRITE) {
+		return -1;
+	}
 	if (semanage_get_trans_lock(sh) < 0) {
 		return -1;
 	}
@@ -775,3 +783,14 @@ static int semanage_direct_list(semanage
         }
 	return retval;
 }
+
+int semanage_direct_access_check(semanage_handle_t *sh) {
+	char polpath[PATH_MAX];	
+
+	snprintf(polpath, PATH_MAX, "%s%s", selinux_path(), sh->conf->store_path);
+
+	if (semanage_check_init(polpath))
+		return -1;
+
+	return semanage_store_access_check(sh);
+}
diff -x.svn -pruN libsemanage/src/direct_api.h libsemanage/src/direct_api.h
--- libsemanage/src/direct_api.h	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/direct_api.h	2006-01-20 09:29:06.000000000 -0500
@@ -37,4 +37,6 @@ int semanage_direct_connect(
 int semanage_direct_is_managed(
 	struct semanage_handle *sh);
 
+int semanage_direct_access_check(struct semanage_handle *sh);
+
 #endif
diff -x.svn -pruN libsemanage/src/handle.c libsemanage/src/handle.c
--- libsemanage/src/handle.c	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/handle.c	2006-01-26 14:55:44.000000000 -0500
@@ -64,6 +64,9 @@ semanage_handle_t *semanage_handle_creat
 	/* By default always reload policy after commit if SELinux is enabled. */
 	sh->do_reload = (is_selinux_enabled() > 0);
 
+	/* By default do not create store */
+	sh->create_store = 0;
+
 	/* Set timeout: some default value for now, later use config */
 	sh->timeout = SEMANAGE_COMMIT_READ_WAIT; 
 
@@ -94,6 +97,19 @@ void semanage_set_reload(semanage_handle
 	return;
 }
 
+void semanage_set_create_store(semanage_handle_t *sh, int create_store) {
+	
+	assert(sh != NULL);
+
+	sh->create_store = create_store;
+	return;
+}
+
+int semanage_is_connected(semanage_handle_t *sh) {
+	assert(sh != NULL);
+	return sh->is_connected;
+}
+
 void semanage_select_store(semanage_handle_t *sh, char *storename,
 			  enum semanage_connect_type storetype) {
 	
@@ -142,6 +158,19 @@ int semanage_connect(semanage_handle_t *
 	return 0;
 }
 
+int semanage_access_check(semanage_handle_t *sh) {
+	assert(sh != NULL);
+	switch (sh->conf->store_type) {
+	case SEMANAGE_CON_DIRECT: 
+		return semanage_direct_access_check(sh);
+	default:
+		return -1;
+	}
+
+	return -1; /* unreachable */
+}
+hidden_def(semanage_access_check)
+
 int semanage_disconnect(semanage_handle_t *sh) {
 	assert(sh != NULL && sh->funcs != NULL && sh->funcs->disconnect != NULL);
 	if (!sh->is_connected) {
diff -x.svn -pruN libsemanage/src/handle.h libsemanage/src/handle.h
--- libsemanage/src/handle.h	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/handle.h	2006-01-26 14:55:44.000000000 -0500
@@ -63,6 +63,8 @@ struct semanage_handle {
 	int do_reload;		/* whether to reload policy after commit */
 	int do_rebuild;		/* whether to rebuild policy if there were no changes */
 	int modules_modified;
+	int create_store;	/* whether to create the store if it does not exist
+				 * this will only have an effect on direct connections */
 
 	/* This timeout is used for transactions and waiting for lock
 	   -1 means wait indefinetely
diff -x.svn -pruN libsemanage/src/handle_internal.h libsemanage/src/handle_internal.h
--- libsemanage/src/handle_internal.h	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/handle_internal.h	2006-01-20 09:29:19.000000000 -0500
@@ -7,5 +7,6 @@
 hidden_proto(semanage_begin_transaction)
 hidden_proto(semanage_handle_destroy)
 hidden_proto(semanage_reload_policy)
+hidden_proto(semanage_access_check)
 
 #endif
diff -x.svn -pruN libsemanage/src/libsemanage.map libsemanage/src/libsemanage.map
--- libsemanage/src/libsemanage.map	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/libsemanage.map	2006-01-26 14:55:44.000000000 -0500
@@ -11,6 +11,7 @@ LIBSEMANAGE_1.0 {
 	  semanage_reload_policy; semanage_set_reload; semanage_set_rebuild;
 	  semanage_user_*; semanage_bool_*; semanage_seuser_*;
 	  semanage_iface_*; semanage_port_*; semanage_context_*;
-	  semanage_fcontext_*;
+	  semanage_fcontext_*; semanage_access_check; semanage_set_create_store;
+	  semanage_is_connected;
   local: *;
 };
diff -x.svn -pruN libsemanage/src/semanage.py libsemanage/src/semanage.py
--- libsemanage/src/semanage.py	2006-01-20 11:30:59.000000000 -0500
+++ libsemanage/src/semanage.py	2006-01-20 11:42:19.000000000 -0500
@@ -87,6 +87,10 @@ semanage_disconnect = _semanage.semanage
 semanage_begin_transaction = _semanage.semanage_begin_transaction
 
 semanage_commit = _semanage.semanage_commit
+SEMANAGE_CAN_READ = _semanage.SEMANAGE_CAN_READ
+SEMANAGE_CAN_WRITE = _semanage.SEMANAGE_CAN_WRITE
+
+semanage_access_check = _semanage.semanage_access_check
 
 semanage_module_install = _semanage.semanage_module_install
 
diff -x.svn -pruN libsemanage/src/semanage_store.c libsemanage/src/semanage_store.c
--- libsemanage/src/semanage_store.c	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/semanage_store.c	2006-01-26 14:55:44.000000000 -0500
@@ -221,6 +221,8 @@ int semanage_create_store(semanage_handl
 	struct stat sb;
 	int mode_mask = R_OK | W_OK | X_OK;
 	const char *path = semanage_files[SEMANAGE_ROOT];
+	int fd;
+
 	if (stat(path, &sb) == -1) {
 		if (errno == ENOENT && create) {
 			if (mkdir(path, S_IRWXU) == -1) {
@@ -278,10 +280,76 @@ int semanage_create_store(semanage_handl
 			return -1;
 		}
 	}
+	path = semanage_files[SEMANAGE_READ_LOCK];
+	if (stat (path, &sb) == -1) {
+		if (errno == ENOENT && create) {
+			if ((fd = creat(path, S_IRUSR | S_IWUSR)) == -1) {
+				ERR(sh, "Could not create lock file at %s.", path);
+				return -2;
+			}
+		}
+		else {
+			ERR(sh, "Could not read lock file at %s.", path);
+			return -1;
+		}
+	}
+	else {
+		if (!S_ISREG(sb.st_mode) || access(path, R_OK | W_OK) == -1) {
+			ERR(sh, "Could not access lock file at %s.", path);
+			return -1;
+		}
+	}
 	return 0;
 }
 
+/* returns <0 if the active store cannot be read or doesn't exist
+ * 0 if the store exists but the lock file cannot be written to
+ * SEMANAGE_CAN_READ if the store can be read and the lock file written to
+ * SEMANAGE_CAN_WRITE if the modules directory and binary policy dir can be written to
+ */
+int semanage_store_access_check(semanage_handle_t *sh) {
+	const char *path;
+	char *path2, *path3;
+	int rc = -1;
+
+	/* read access on active store */
+	path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_TOPLEVEL);
+	if (access(path, R_OK | X_OK) != 0)
+		goto out;
+
+	/* we can read the active store meaning it is managed
+	 * so now we return 0 to indicate no error */
+	rc = 0;
+
+	/* read/write access on lock file required for reading */
+	path = semanage_files[SEMANAGE_READ_LOCK];
+	if (access(path, R_OK | W_OK) != 0)
+		goto out;
+
+	/* everything needed for reading has been checked */
+	rc = SEMANAGE_CAN_READ;
 
+	/* check the modules directory */
+	path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_MODULES);
+	if (access(path, R_OK | W_OK | X_OK) != 0)
+		goto out;
+
+	/* check the binary policy install directory */
+	path = selinux_binary_policy_path();
+	path2 = strdup(path);
+	path3 = dirname(path2);
+
+	if ((access(path3, R_OK | W_OK | X_OK)) != 0) {
+		free(path2);
+		goto out;
+	}
+	free(path2);
+	
+	rc = SEMANAGE_CAN_WRITE;
+
+out:
+	return rc;
+}
 
 /********************* other I/O functions *********************/
 
diff -x.svn -pruN libsemanage/src/semanage_store.h libsemanage/src/semanage_store.h
--- libsemanage/src/semanage_store.h	2006-01-18 11:56:33.000000000 -0500
+++ libsemanage/src/semanage_store.h	2006-01-19 15:11:05.000000000 -0500
@@ -99,5 +99,6 @@ int semanage_verify_modules(
 int semanage_verify_linked(semanage_handle_t *sh);
 int semanage_verify_kernel(semanage_handle_t *sh);
 int semanage_split_fc(semanage_handle_t *sh);
+int semanage_store_writable(semanage_handle_t *sh);
 
 #endif
diff -x.svn -pruN libsemanage/src/semanageswig_wrap.c libsemanage/src/semanageswig_wrap.c
--- libsemanage/src/semanageswig_wrap.c	2006-01-20 11:30:59.000000000 -0500
+++ libsemanage/src/semanageswig_wrap.c	2006-01-20 11:42:19.000000000 -0500
@@ -2398,6 +2398,26 @@ static PyObject *_wrap_semanage_commit(P
 }
 
 
+static PyObject *_wrap_semanage_access_check(PyObject *self, PyObject *args) {
+    PyObject *resultobj;
+    semanage_handle_t *arg1 = (semanage_handle_t *) 0 ;
+    int result;
+    PyObject * obj0 = 0 ;
+    
+    if(!PyArg_ParseTuple(args,(char *)"O:semanage_access_check",&obj0)) goto fail;
+    SWIG_Python_ConvertPtr(obj0, (void **)&arg1, SWIGTYPE_p_semanage_handle, SWIG_POINTER_EXCEPTION | 0);
+    if (SWIG_arg_fail(1)) SWIG_fail;
+    result = (int)semanage_access_check(arg1);
+    
+    {
+        resultobj = SWIG_From_int((int)(result)); 
+    }
+    return resultobj;
+    fail:
+    return NULL;
+}
+
+
 static PyObject *_wrap_semanage_module_install(PyObject *self, PyObject *args) {
     PyObject *resultobj;
     semanage_handle_t *arg1 = (semanage_handle_t *) 0 ;
@@ -7472,6 +7492,7 @@ static PyMethodDef SwigMethods[] = {
 	 { (char *)"semanage_disconnect", _wrap_semanage_disconnect, METH_VARARGS, NULL},
 	 { (char *)"semanage_begin_transaction", _wrap_semanage_begin_transaction, METH_VARARGS, NULL},
 	 { (char *)"semanage_commit", _wrap_semanage_commit, METH_VARARGS, NULL},
+	 { (char *)"semanage_access_check", _wrap_semanage_access_check, METH_VARARGS, NULL},
 	 { (char *)"semanage_module_install", _wrap_semanage_module_install, METH_VARARGS, NULL},
 	 { (char *)"semanage_module_upgrade", _wrap_semanage_module_upgrade, METH_VARARGS, NULL},
 	 { (char *)"semanage_module_install_base", _wrap_semanage_module_install_base, METH_VARARGS, NULL},
@@ -8139,6 +8160,12 @@ SWIGEXPORT(void) SWIG_init(void) {
         PyDict_SetItemString(d,"SEMANAGE_CON_POLSERV_REMOTE", SWIG_From_int((int)(SEMANAGE_CON_POLSERV_REMOTE))); 
     }
     {
+        PyDict_SetItemString(d,"SEMANAGE_CAN_READ", SWIG_From_int((int)(1))); 
+    }
+    {
+        PyDict_SetItemString(d,"SEMANAGE_CAN_WRITE", SWIG_From_int((int)(2))); 
+    }
+    {
         PyDict_SetItemString(d,"SEMANAGE_PROTO_UDP", SWIG_From_int((int)(0))); 
     }
     {
diff -x.svn -pruN policycoreutils/semanage/semanage policycoreutils/semanage/semanage
--- policycoreutils/semanage/semanage	2006-01-26 14:52:07.000000000 -0500
+++ policycoreutils/semanage/semanage	2006-01-26 14:54:10.000000000 -0500
@@ -24,9 +24,6 @@ import os, sys, getopt
 import seobject
 
 if __name__ == '__main__':
-	if os.getuid() > 0 or os.geteuid() > 0:
-		print "You must be root to run %s." % sys.argv[0]
-		sys.exit(0)
 
 	def usage(message = ""):
 		print '\
diff -x.svn -pruN policycoreutils/semanage/seobject.py policycoreutils/semanage/seobject.py
--- policycoreutils/semanage/seobject.py	2006-01-20 11:31:07.000000000 -0500
+++ policycoreutils/semanage/seobject.py	2006-01-26 15:11:45.000000000 -0500
@@ -143,10 +143,19 @@ class semanageRecords:
 	def __init__(self):
 		self.sh = semanage_handle_create()
 		self.semanaged = semanage_is_managed(self.sh)
-		if self.semanaged:
-			rc = semanage_connect(self.sh)
-			if rc < 0:
-				raise ValueError("Could not establish semanage connection")
+		if not self.semanaged:
+			semanage_handle_destroy(self.sh)
+			raise ValueError("SELinux policy is not managed or store cannot be accessed.")
+
+		rc = semanage_access_check(self.sh)
+		if rc < SEMANAGE_CAN_READ:
+			semanage_handle_destroy(self.sh)
+			raise ValueError("Cannot read policy store.")
+
+		rc = semanage_connect(self.sh)
+		if rc < 0:
+			semanage_handle_destroy(self.sh)
+			raise ValueError("Could not establish semanage connection")
 
 class loginRecords(semanageRecords):
 	def __init__(self):
diff -x.svn -pruN policycoreutils/semodule/semodule.c policycoreutils/semodule/semodule.c
--- policycoreutils/semodule/semodule.c	2006-01-06 10:02:34.000000000 -0500
+++ policycoreutils/semodule/semodule.c	2006-01-26 15:11:45.000000000 -0500
@@ -41,6 +41,7 @@ static int num_commands = 0;
 static int verbose;
 static int reload;
 static int no_reload;
+static int create_store;
 static int build;
 
 static semanage_handle_t *sh = NULL;
@@ -169,9 +170,10 @@ static void parse_command_line(int argc,
         verbose = 0;
 	reload = 0;
 	no_reload = 0;
+	create_store = 0;
         while ((i = getopt_long(argc, argv, "s:b:hi:lvqr:u:RnB", opts, NULL)) != -1) {
                 switch (i) {
-                case 'b': set_mode(BASE_M, optarg); break;
+                case 'b': set_mode(BASE_M, optarg); create_store = 1; break;
                 case 'h': usage(argv[0]); exit(0);
                 case 'i': set_mode(INSTALL_M, optarg); break;
                 case 'l': set_mode(LIST_M, NULL); break;
@@ -233,6 +235,21 @@ int main(int argc, char *argv[]) {
 		 * location */
 		semanage_select_store(sh, store, SEMANAGE_CON_DIRECT);
 	}
+	
+	/* if installing base module create store if necessary, for bootstrapping */
+	semanage_set_create_store(sh, create_store);
+	
+	if (!create_store) {
+		if (!semanage_is_managed(sh)) {
+			fprintf(stderr, "%s: SELinux policy is not managed or store cannot be accessed.\n", argv[0]);
+			goto cleanup;
+		}
+	
+		if (semanage_access_check(sh) < SEMANAGE_CAN_READ) {
+			fprintf(stderr, "%s: Cannot read policy store.\n", argv[0]);
+			goto cleanup;
+		}
+	}
 
         if ((result = semanage_connect(sh)) < 0) {
                 fprintf(stderr, "%s:  Could not connect to policy handler\n", argv[0]);
@@ -359,9 +376,11 @@ int main(int argc, char *argv[]) {
         status = EXIT_SUCCESS;
 
  cleanup:
-        if (semanage_disconnect(sh) < 0) {
-		fprintf(stderr, "%s:  Error disconnecting\n", argv[0]);
-        }
+	if (semanage_is_connected(sh)) {
+	        if (semanage_disconnect(sh) < 0) {
+			fprintf(stderr, "%s:  Error disconnecting\n", argv[0]);
+		}
+	}
         semanage_handle_destroy(sh);
         cleanup();
         exit(status);

Re: [PATCH] libsemanage/semanage - permission check for semanage

[Stephen Smalley]

On Thu, 2006-01-26 at 15:40 -0500, Joshua Brindle wrote:
> This patch adds semanage_set_create_store which is callable by the
> client to create the store. Set to 1 by semodule when called with -b.
> Otherwise the same access checks as before are done.
> 
> Also adds semanage_is_connected to avoid an assertion failure in
> semodule if an error occurs before connecting.

Merged as of libsemanage 1.5.17 and policycoreutils 1.29.13, with the
following modifications:
- swigify/pywrap rebuild to pick up additional interfaces,
- dropped binary policy directory writability check,
- replaced obsolete semanage_store_writable prototype with
semanage_store_access_check prototype.

Also added a distclean target to the libsemanage top Makefile, and made
the relabel target apply restorecon to the .so file like the other
library Makefiles.

-- 
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: [PATCH] libsemanage/semanage - permission check for semanage

[Joshua Brindle]

Stephen Smalley wrote:
> On Thu, 2006-01-26 at 15:40 -0500, Joshua Brindle wrote:
> 
>>This patch adds semanage_set_create_store which is callable by the
>>client to create the store. Set to 1 by semodule when called with -b.
>>Otherwise the same access checks as before are done.
>>
>>Also adds semanage_is_connected to avoid an assertion failure in
>>semodule if an error occurs before connecting.
> 
> 
> Merged as of libsemanage 1.5.17 and policycoreutils 1.29.13, with the
> following modifications:
> - swigify/pywrap rebuild to pick up additional interfaces,
> - dropped binary policy directory writability check,

ah, sorry, I meant to remove that

> - replaced obsolete semanage_store_writable prototype with
> semanage_store_access_check prototype.
>

> Also added a distclean target to the libsemanage top Makefile, and made
> the relabel target apply restorecon to the .so file like the other
> library Makefiles.
> 

Thanks

--
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: [PATCH] libsemanage/semanage - permission check for semanage

[Joshua Brindle]

Stephen Smalley wrote:
> On Mon, 2006-01-23 at 10:40 -0500, Joshua Brindle wrote:
> 
>>speaking of the create_store code path, That function is no longer in 
>>use and it was primarily used for bootstrapping non-managed systems so I 
>>didn't remove it yet but we can probably just require the package 
>>manager or make scripts to initialize the store and remove this from the 
>>library entirely.
> 
> 
> Hmm...I missed that side effect.  Moving aside my module store and
> trying to re-install the base module from /usr/share/selinux does indeed
> fail against the new libsemanage (upon connect).  So the policy package
> would have to pre-create the directory tree and the lock files so that
> the connect could succeed?  I'm not sure that is truly what we want.
> 

Mainly I changed it because if you didn't have write access it would 
spam the user with warnings and return -1. create_store is currently 
checking for R_OK | W_OK | X_OK access even if it is not necessary to 
make the directories. We could change this but that might have other 
side effects.

Further, since we don't know if the system is unmanaged or the modules 
directory is just inaccessible you can't even start a transaction right 
now (I added  if (semanage_access_check(sh) != SEMANAGE_CAN_WRITE) 
return -1 to direct_begintrans).

The main reason for this patch was to let the client give the user nice 
errors in these access cases since Dan's intention by checking for UID 
== 0 was to not spam the user with libsemanage debug messages. I'm not 
sure how to maintain the current behaviour while keeping access messages 
reasonably quiet.

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