[ SEMANAGE ] Fix MLS parsing of users

[ SEMANAGE ] Fix MLS parsing of users

[Ivan Gyurdiev]

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

Okay, I hadn't really tested MLS users very well. This patch corrects 
that oversight, and makes commit() pass on an MLS/MCS enabled system.

Changes:

- parse_filter_space_until is hopelessly broken - there's at least 2 
segfault bugs in it - delete the whole function, and handle MLS for 
users the same way I handle it for seusers - disallow multiline and 
spaces. This whole approach of looking for the "range" substring at the 
end is wrong in the first place, and genusers shouldn't be using it. 
Maybe I'll implement this properly in the future, but right now it 
doesn't seem too important - will keep MLS without spaces on a single line.

- the check whether "level" is present is backwards - fix it





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

diff -Naurp --exclude CVS --exclude ChangeLog --exclude VERSION --exclude Makefile old/libsemanage/src/parse_utils.c new/libsemanage/src/parse_utils.c
--- old/libsemanage/src/parse_utils.c	2005-10-31 22:31:55.000000000 -0500
+++ new/libsemanage/src/parse_utils.c	2005-10-31 23:04:25.000000000 -0500
@@ -151,7 +151,8 @@ int parse_assert_noeof(
 	parse_info_t* info) {
 
 	if (!info->ptr) {
-		ERR(handle, "unexpected end of file");
+		ERR(handle, "unexpected end of file (%s: %u)",
+			info->filename, info->lineno);
 		return STATUS_ERR;
 	}
 
@@ -233,59 +234,6 @@ int parse_optional_str(parse_info_t* inf
 	}
 }
 
-char* parse_filter_space_until(
-	semanage_handle_t* handle, 
-	parse_info_t* info, 
-	const char* substr) {
-
-	char* buffer = NULL, *wr, *tmp;
-	int len = strlen(substr);
-	int used = 0;
-	int csize = 0;
-
-	wr = buffer;
-	do {
-		/* If content is not a space, copy to buffer */
-		if (!isspace(info->ptr)) {
-
-			/* If we're out of space, increase by 15 */
-			if (used + 1 >= csize) {
-				csize += 15;
-				tmp = realloc(buffer, csize);
-				if (!tmp)
-					goto omem;
-				buffer = tmp;
-			}
-			*wr++ = *info->ptr;
-			used++;
-		}
-		info->ptr++;
-
-		if (parse_skip_space(handle, info) < 0)
-			goto err;
-		if (parse_assert_noeof(handle, info) < 0)
-			goto err;
-
-	} while(!strncasecmp(info->ptr, substr, len));
-
-	if (!buffer) {
-		buffer = malloc(1);
-		if (!buffer)
-			goto omem;
-	}
-
-	*wr = '\0';
-
-	return buffer;
-
-	omem:
-	ERR(handle, "out of memory, could not allocate buffer");
-
-	err:
-	free(buffer);
-	return NULL;
-}
-
 int parse_fetch_string(
 	semanage_handle_t* handle,
 	parse_info_t* info, 
diff -Naurp --exclude CVS --exclude ChangeLog --exclude VERSION --exclude Makefile old/libsemanage/src/parse_utils.h new/libsemanage/src/parse_utils.h
--- old/libsemanage/src/parse_utils.h	2005-10-31 21:52:14.000000000 -0500
+++ new/libsemanage/src/parse_utils.h	2005-10-31 22:53:40.000000000 -0500
@@ -83,15 +83,6 @@ extern int parse_optional_str(
 	parse_info_t* info, 
 	const char* str);
 
-/* Buffer a string, filtering all 
- * whitespace, until substring is encountered,
- * at which point return the buffered string.
- * This function will work on multiple lines */
-extern char* parse_filter_space_until(
-	semanage_handle_t* handle,
-	parse_info_t* info,
-	const char* substr);
-
 /* Extract the next string (delimited by 
  * whitespace), and move the read pointer past it */
 extern int parse_fetch_string(
diff -Naurp --exclude CVS --exclude ChangeLog --exclude VERSION --exclude Makefile old/libsemanage/src/users_file.c new/libsemanage/src/users_file.c
--- old/libsemanage/src/users_file.c	2005-10-31 22:31:56.000000000 -0500
+++ new/libsemanage/src/users_file.c	2005-10-31 23:04:38.000000000 -0500
@@ -70,7 +70,7 @@ static int user_parse(
 	semanage_user_t* user) {
 
 	int islist = 0;
-	char* mls = NULL;
+	char* str = NULL;
 	char* start;
 	char* name_str = NULL;
 
@@ -164,35 +164,39 @@ static int user_parse(
 			goto err;
 		if (parse_assert_noeof(handle, info) < 0)
 			goto err;
-		if (parse_optional_str(info, "level") != STATUS_NODATA)
+		if (parse_optional_str(info, "level") == STATUS_NODATA)
 			goto semicolon;
 		if (parse_assert_space(handle, info) < 0)
 			goto err;
 		if (parse_assert_noeof(handle, info) < 0)
 			goto err;
 
-		mls = parse_filter_space_until(handle, info, "range");
-		if (!mls)
+		/* NOTE: does not allow spaces/multiline */
+		if (parse_fetch_string(handle, info, &str) < 0)
 			goto err;
-		if (semanage_user_set_mlslevel(handle, user, mls) < 0)
+		if (semanage_user_set_mlslevel(handle, user, str) < 0)
 			goto err;
-		free(mls);
+		free(str);
+		str = NULL;
 
 		/* Parse range header */
+		if (parse_assert_space(handle, info) < 0)
+			goto err;
 		if (parse_assert_str(handle, info, "range") < 0)
-                       goto err;
-
+			goto err;
 		if (parse_assert_space(handle, info) < 0)
 			goto err;
 		if (parse_assert_noeof(handle, info) < 0)
 			goto err;
 
-		mls = parse_filter_space_until(handle, info, ";");
-		if (!mls)
+		/* NOTE: does not allow spaces/multiline */
+		if (parse_fetch_string_until(handle, info, &str, ';') < 0)
 			goto err;
-		if (semanage_user_set_mlsrange(handle, user, mls) < 0)
+		if (semanage_user_set_mlsrange(handle, user, str) < 0)
 			goto err;
-		free(mls);
+
+		free(str);
+		str = NULL;
 	}
 
 	/* Check for semicolon */
@@ -213,7 +217,7 @@ static int user_parse(
 
 	err:
 	ERR(handle, "could not parse user record");
-	free(mls);
+	free(str);
 	parse_dispose_line(info);
 	return STATUS_ERR;
 }

Re: [ SEMANAGE ] Fix MLS parsing of users

[Ivan Gyurdiev]

By the way, expanding the module (sepol_expand_module) takes an 
unusually long time to run... commit is very slow (and this is on an 
Athlon3k computer)....

1. Time to modify seuser things: 0.5 sec
2. Time to call sepol_expand_module: 9 sec
3. Time to merge and commit other policy components, and load the 
policy: 1 sec

You can see why I want (2), and most of (3) skipped in the case of 
seusers...need to add tracking of when the policy is modified.


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

[ SEMANAGE ] Some seusers mapping validation

[Ivan Gyurdiev]

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


>
> You can see why I want (2), and most of (3) skipped in the case of 
> seusers...need to add tracking of when the policy is modified.
Well... with the attached patch I need a policydb, regardless of whether 
modifications occured.. otoh if no modifications, then I don't have to 
call expand, which takes so long... can go through the policydb_cache 
function instead (not sure if that's any faster, however..)

Changes:
- add some basic validation for seusers - abort the commit if the sename 
is invalid (Selinux user does not exist). This will also prevent 
deletion of users without deleting/changing the corresponding mappings 
in the seusers file - it works out rather nicely.

We might also want to validate the MLS range, and the Unix name. I am 
not clear on how to validate the MLS range - what's happening with the 
old (local.users) MLS range? Is it deprecated? Are they supposed to 
match? How to handle this?





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

diff -Naurp --exclude CVS --exclude ChangeLog --exclude VERSION --exclude Makefile old/libsemanage/src/policy_components.c new/libsemanage/src/policy_components.c
--- old/libsemanage/src/policy_components.c	2005-10-31 21:52:14.000000000 -0500
+++ new/libsemanage/src/policy_components.c	2005-11-01 00:37:31.000000000 -0500
@@ -2,6 +2,7 @@
 #include "handle.h"
 #include "database.h"
 #include "modules.h"
+#include "seusers.h"
 #include "debug.h"
 
 #define MODE_SET    1
@@ -125,6 +126,10 @@ int semanage_commit_components(
 		semanage_seuser_dbase(handle)
 	};
 
+	/* Validate seusers */
+	if (semanage_seuser_validate(handle) < 0)
+		goto err;
+
 	for (i = 0; i < CCOUNT; i++) {
 		/* Flush to disk */
 		if (components[i]->dtable->flush(
diff -Naurp --exclude CVS --exclude ChangeLog --exclude VERSION --exclude Makefile old/libsemanage/src/seusers.c new/libsemanage/src/seusers.c
--- old/libsemanage/src/seusers.c	2005-10-31 11:09:39.000000000 -0500
+++ new/libsemanage/src/seusers.c	2005-11-01 00:46:31.000000000 -0500
@@ -5,9 +5,12 @@ typedef semanage_seuser_t record_t;
 #define DBASE_RECORD_DEFINED
 
 #include <stddef.h>
-#include <semanage/seusers.h>
+#include <semanage/user_record.h>
+#include <semanage/users_policy.h>
+#include "seusers.h"
 #include "handle.h"
 #include "database.h"
+#include "debug.h"
 
 int semanage_seuser_add(
 	semanage_handle_t* handle,
@@ -88,3 +91,59 @@ int semanage_seuser_list(
 	dbase_config_t* dconfig = semanage_seuser_dbase(handle);
 	return dbase_list(handle, dconfig, records, count);
 }
+
+
+struct validate_handler_arg {
+	semanage_handle_t* handle;
+};
+
+static int validate_handler(
+	semanage_seuser_t* seuser,
+	void* varg) {
+
+	struct validate_handler_arg* arg = 
+		(struct validate_handler_arg*) varg;
+
+	const char* name = semanage_seuser_get_name(seuser);
+	const char* sename = semanage_seuser_get_sename(seuser);
+	const char* mls_range = semanage_seuser_get_mlsrange(seuser);
+
+	semanage_user_key_t* key = NULL;
+	int exists;
+	if (semanage_user_key_create(arg->handle, sename, &key) < 0)
+		goto err;
+
+	if (semanage_user_exists(arg->handle, key, &exists) < 0)
+		goto err;
+
+	if (!exists) {
+		ERR(arg->handle, "selinux user %s does not exist", sename);
+		goto invalid;
+	}
+
+	/* FIXME: check unix user? */
+	/* FIXME: add MLS checks */
+
+	semanage_user_key_free(key);
+	return 0;
+
+	err:
+	ERR(arg->handle, "could not check if the seuser mapping "
+		"%s -> (%s, %s) is valid", name, sename, mls_range);
+	semanage_user_key_free(key);
+	return -1;
+
+	invalid:
+	ERR(arg->handle, "seuser mapping %s -> (%s, %s) is invalid",
+		name, sename, mls_range);
+	semanage_user_key_free(key);
+	return -1;
+}
+
+int semanage_seuser_validate(
+	semanage_handle_t* handle) {
+
+	struct validate_handler_arg arg;
+	arg.handle = handle;
+	return semanage_seuser_iterate(handle, validate_handler, &arg);
+}
diff -Naurp --exclude CVS --exclude ChangeLog --exclude VERSION --exclude Makefile old/libsemanage/src/seusers.h new/libsemanage/src/seusers.h
--- old/libsemanage/src/seusers.h	1969-12-31 19:00:00.000000000 -0500
+++ new/libsemanage/src/seusers.h	2005-11-01 00:37:02.000000000 -0500
@@ -0,0 +1,9 @@
+#ifndef _SEUSERS_INTERNAL_H_
+#define _SEUSERS_INTERNAL_H_
+
+#include <semanage/seusers.h>
+
+extern int semanage_seuser_validate(
+	semanage_handle_t* handle);
+
+#endif

Re: [ SEMANAGE ] Some seusers mapping validation

[Ivan Gyurdiev]

> +extern int semanage_seuser_validate(
> +	semanage_handle_t* handle);
>   
Doh... will be exported by the map wildcard...
Should I get rid of wildcards?


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

Re: [ SEMANAGE ] Fix MLS parsing of users

[Stephen Smalley]

On Mon, 2005-10-31 at 23:38 -0500, Ivan Gyurdiev wrote:
> By the way, expanding the module (sepol_expand_module) takes an 
> unusually long time to run... commit is very slow (and this is on an 
> Athlon3k computer)....
> 
> 1. Time to modify seuser things: 0.5 sec
> 2. Time to call sepol_expand_module: 9 sec
> 3. Time to merge and commit other policy components, and load the 
> policy: 1 sec
> 
> You can see why I want (2), and most of (3) skipped in the case of 
> seusers...need to add tracking of when the policy is modified.

If you pass a '0' check flag to sepol_expand_module to suppress
assertion and hierarchy checking, it is much faster.   For development,
you can just echo "expand-check = 0" >> /etc/selinux/semanage.conf to
turn it off.  For production, you could make it a flag parameter to
semanage_expand_sandbox rather than just a config setting.

-- 
Stephen Smalley
National Security Agency


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

Re: [ SEMANAGE ] Some seusers mapping validation

[Stephen Smalley]

On Tue, 2005-11-01 at 01:03 -0500, Ivan Gyurdiev wrote:
> > +extern int semanage_seuser_validate(
> > +	semanage_handle_t* handle);
> >   
> Doh... will be exported by the map wildcard...
> Should I get rid of wildcards?

Or we can mark it hidden, once I add dso.h.

-- 
Stephen Smalley
National Security Agency


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

Re: [ SEMANAGE ] Some seusers mapping validation

[Stephen Smalley]

On Tue, 2005-11-01 at 00:59 -0500, Ivan Gyurdiev wrote:
> We might also want to validate the MLS range, and the Unix name. I am 
> not clear on how to validate the MLS range - what's happening with the 
> old (local.users) MLS range? Is it deprecated? Are they supposed to 
> match? How to handle this?

users.local allows you to define additional SELinux users and authorize
them for role sets and ranges.  seusers allows you to map Linux users to
SELinux users defined in either the policy modules or users.local, and
to assign the Linux user a subset of the range authorized for the
SELinux user.  Hence, validation of the MLS range in seusers would
consist of:
- validating the range by itself as usual,
- validating that the range is a subset of the range authorized for the
SELinux user.

-- 
Stephen Smalley
National Security Agency


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

Re: [ SEMANAGE ] Some seusers mapping validation

[Stephen Smalley]

On Tue, 2005-11-01 at 00:59 -0500, Ivan Gyurdiev wrote:
> >
> > You can see why I want (2), and most of (3) skipped in the case of 
> > seusers...need to add tracking of when the policy is modified.
> Well... with the attached patch I need a policydb, regardless of whether 
> modifications occured.. otoh if no modifications, then I don't have to 
> call expand, which takes so long... can go through the policydb_cache 
> function instead (not sure if that's any faster, however..)

All five patches merged as of libsemanage 1.3.40, with the minor change
to make errors upon copying of seusers non-fatal.

-- 
Stephen Smalley
National Security Agency


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

Re: [ SEMANAGE ] Some seusers mapping validation

[Ivan Gyurdiev]

>
> users.local allows you to define additional SELinux users and authorize
> them for role sets and ranges.  seusers allows you to map Linux users to
> SELinux users defined in either the policy modules or users.local, and
> to assign the Linux user a subset of the range authorized for the
> SELinux user.  Hence, validation of the MLS range in seusers would
> consist of:
> - validating the range by itself as usual,
> - validating that the range is a subset of the range authorized for the
> SELinux user.
>   

This requires additional interfaces in context.c that do not currently 
exist...


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