[PATCH] libselinux

[PATCH] libselinux

[Jonathan Kim]

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

This patch is for handling correctly when label portion is removed from
seusers file. 
Will refer to *.users file instead of asking role:type:level to user as
expected.

 <<libselinux_seusers.patch>> 

-----------------------------------
Jonghoon Jonathan
Secure Systems Engineer

Trusted Computer Solutions
121 W Goose Alley
Urbana, IL  61801

http://www.TrustedCS.com

V:  217.384.0028  ext.16
F:  217.384.0288


[-- Attachment #2: libselinux_seusers.patch --]
[-- Type: application/octet-stream, Size: 464 bytes --]

diff -Nur libselinux-1.27.17.orig/src/seusers.c libselinux-1.27.17/src/seusers.c
--- libselinux-1.27.17.orig/src/seusers.c	2005-10-27 17:50:31.000000000 +0000
+++ libselinux-1.27.17/src/seusers.c	2005-11-02 17:56:32.574237594 +0000
@@ -63,9 +63,11 @@
 		end++;
 	*end = 0;
 
-	level = strdup(start);
-	if (!level)
-		goto err;
+	if (*start && !isspace(*start)) {
+		level = strdup(start);
+		if (!level)
+			goto err;
+	}
 
 out:
 	free(newbuf);

Re: [PATCH] libselinux

[Stephen Smalley]

On Thu, 2005-11-03 at 14:33 -0500, Jonathan Kim wrote:
> This patch is for handling correctly when label portion is removed from
> seusers file. 
> Will refer to *.users file instead of asking role:type:level to user as
> expected.

I'm not sure that this is what you want.  Keep in mind that seusers is
intended to allow you to assign ranges and role sets (aka SELinux users)
to individual Linux users without requiring any modification to SELinux
policy (even at policy load time, which is going away with the advent of
libsemanage; SETLOCALDEFS=0 should become the norm
for /etc/selinux/config soon).  If you don't want to use seusers at all,
you can certainly omit it from your policy, in which case things will
fall through to the usual logic, and you can even do that per-user by
omitting an entry for that user entirely from seusers, but I'm not clear
on why you would have an entry for the Linux user but no level there -
that means that you still need per-Linux user entries in your policy for
the range authorizations, which defeats the purpose of having the user
in seusers at all.

-- 
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] libselinux

[Stephen Smalley]

On Thu, 2005-11-03 at 15:10 -0500, Stephen Smalley wrote:
> On Thu, 2005-11-03 at 14:33 -0500, Jonathan Kim wrote:
> > This patch is for handling correctly when label portion is removed from
> > seusers file. 
> > Will refer to *.users file instead of asking role:type:level to user as
> > expected.
> 
> I'm not sure that this is what you want.  Keep in mind that seusers is
> intended to allow you to assign ranges and role sets (aka SELinux users)
> to individual Linux users without requiring any modification to SELinux
> policy (even at policy load time, which is going away with the advent of
> libsemanage; SETLOCALDEFS=0 should become the norm
> for /etc/selinux/config soon).  If you don't want to use seusers at all,
> you can certainly omit it from your policy, in which case things will
> fall through to the usual logic, and you can even do that per-user by
> omitting an entry for that user entirely from seusers, but I'm not clear
> on why you would have an entry for the Linux user but no level there -
> that means that you still need per-Linux user entries in your policy for
> the range authorizations, which defeats the purpose of having the user
> in seusers at all.

Just to note:  I'm not opposed to the patch per se (although it might be
better to treat this as an error condition rather than falling through
to the base user definitions, as they may give wider access then you
want since multiple Linux users may be mapped to a single SELinux user),
but I'm unclear about why one would use an empty level in this manner in
seusers versus just omitting the entry for the user altogether if you
want the fallback behavior.

-- 
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] libselinux

[Stephen Smalley]

On Thu, 2005-11-03 at 15:33 -0500, Stephen Smalley wrote:
> Just to note:  I'm not opposed to the patch per se (although it might be
> better to treat this as an error condition rather than falling through
> to the base user definitions, as they may give wider access then you
> want since multiple Linux users may be mapped to a single SELinux user),
> but I'm unclear about why one would use an empty level in this manner in
> seusers versus just omitting the entry for the user altogether if you
> want the fallback behavior.

The other point to keep in mind here is that there are plans to create
management utilities for seusers (via libsemanage) and that those
utilities are going to be instrumented for audit as part of the
LSPP/RBAC effort, whereas local.users is largely unused except by
SELinux developers even now and is even less likely to be used going
forward now that seusers exists.  The expectation is that the base
policy (module) will contain the base set of SELinux user identities
(which will correspond to distinct role sets defined in the policy and
typically be authorized for the full range), and seusers will handle
mapping each Linux user to a role set (aka SELinux user identity) and to
an authorized range.  The user ranges in the SELinux policy will then be
largely unused (typically just the full range), as seusers will be what
governs the clearance/high assigned to the user session based on the
Linux 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.

[ SEMANAGE] Resync seuser parser

[Ivan Gyurdiev]

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

Must keep the parsers in sync (or at least, the semanage parser should 
supercede the other one). Patch attached.

This is now perfectly valid on my machine (see below).
Loaded correctly, modified correcly, written back out in the same order, 
and straightened out into something sane.

test_user:root
                phantom:

        phantom:s0

__default__

        :user_u root:   root       :


s0


Test output
-----
There are 4 seusers before the transaction.

Now testing list...
Name: root, Seuser: root, MLS: s0
Name: __default__, Seuser: user_u, MLS: (null)
Name: phantom, Seuser: phantom, MLS: s0
Name: test_user, Seuser: root, MLS: (null)

Now testing iterate...
Name: root, Seuser: root, MLS: s0
Name: __default__, Seuser: user_u, MLS: (null)
Name: phantom, Seuser: phantom, MLS: s0
Name: test_user, Seuser: root, MLS: (null)

In-Transaction pre-commit iterate (should show modifications)..
Name: root, Seuser: root, MLS: s0
Name: __default__, Seuser: user_u, MLS: (null)
Name: phantom, Seuser: phantom, MLS: s0
Name: test_user, Seuser: root, MLS: s0:c5,c6-s2:c1,c3,c5

There are 4 seusers after the transaction

Now testing list
Name: root, Seuser: root, MLS: s0
Name: __default__, Seuser: user_u, MLS: (null)
Name: phantom, Seuser: phantom, MLS: s0
Name: test_user, Seuser: root, MLS: s0:c5,c6-s2:c1,c3,c5

Now testing iterate...
Name: root, Seuser: root, MLS: s0
Name: __default__, Seuser: user_u, MLS: (null)
Name: phantom, Seuser: phantom, MLS: s0
Name: test_user, Seuser: root, MLS: s0:c5,c6-s2:c1,c3,c5

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

diff -Naurp --exclude CVS --exclude ChangeLog --exclude VERSION --exclude Makefile old/libsemanage/src/seusers_file.c new/libsemanage/src/seusers_file.c
--- old/libsemanage/src/seusers_file.c	2005-11-03 12:48:03.000000000 -0500
+++ new/libsemanage/src/seusers_file.c	2005-11-03 16:21:49.000000000 -0500
@@ -81,8 +81,8 @@ static int seuser_parse(
 	if (is_selinux_mls_enabled()) {
 		if (parse_skip_space(handle, info) < 0)
 			goto err;
-		if (parse_assert_ch(handle, info, ':') < 0)
-			goto err;
+		if (parse_optional_ch(info, ':') == STATUS_NODATA)
+			goto out;
 		if (parse_skip_space(handle, info) < 0)
 			goto err;
 
@@ -95,7 +95,8 @@ static int seuser_parse(
 		free(str);
 		str = NULL;
 	} 
-
+	
+	out:
 	return STATUS_SUCCESS;
 
 	last:

Re: [ SEMANAGE] Resync seuser parser

[Ivan Gyurdiev]

This patch is possibly wrong...  do you want the ':MLS' skipped, or only 
the 'MLS' (keeping the semicolon). If the second, then I need to change 
my patch a bit (but we still need to keep the parsers synchronized).

--
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] Resync seuser parser

[Ivan Gyurdiev]

Ivan Gyurdiev wrote:
> This patch is possibly wrong...  do you want the ':MLS' skipped, or 
> only the 'MLS' (keeping the semicolon). If the second, then I need to 
> change my patch a bit (but we still need to keep the parsers 
> synchronized).
Actually on second thought I can't handle the second case, and keep 
multiline support at the same time...do you really need that?

--
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] Resync seuser parser

[Stephen Smalley]

On Fri, 2005-11-04 at 03:15 -0500, Ivan Gyurdiev wrote:
> Ivan Gyurdiev wrote:
> > This patch is possibly wrong...  do you want the ':MLS' skipped, or 
> > only the 'MLS' (keeping the semicolon). If the second, then I need to 
> > change my patch a bit (but we still need to keep the parsers 
> > synchronized).
> Actually on second thought I can't handle the second case, and keep 
> multiline support at the same time...do you really need that?

I don't see why it is needed; it seems like one would omit the separator
as well if no level was being specified.

-- 
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] Resync seuser parser

[Ivan Gyurdiev]

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


> I don't see why it is needed; it seems like one would omit the separator
> as well if no level was being specified.
>   



[-- Attachment #2: libselinux.missing_colon.diff --]
[-- Type: text/x-patch, Size: 1268 bytes --]

diff -Naurp --exclude CVS --exclude ChangeLog --exclude VERSION --exclude Makefile old/libselinux/src/seusers.c new/libselinux/src/seusers.c
--- old/libselinux/src/seusers.c	2005-11-04 09:33:06.000000000 -0500
+++ new/libselinux/src/seusers.c	2005-11-04 09:31:54.000000000 -0500
@@ -19,6 +19,7 @@ static int process_seusers(const char *b
 	char *newbuf = strdup(buffer);
 	char *luser = NULL, *seuser = NULL, *level = NULL;
 	char *start, *end;
+	int mls_found = 1;
 
 	if (!newbuf)
 		goto err;
@@ -42,8 +43,8 @@ static int process_seusers(const char *b
 	start = end+1;
 	end = strchr(start, ':');
 	if (!end) {
-		if (mls_enabled)
-			goto err; /* no MLS level and MLS is enabled */
+		mls_found = 0;
+
 		/* MLS is disabled, so :level suffix not required. */
 		end = start;
 		while (*end && !isspace(*end))
@@ -55,7 +56,7 @@ static int process_seusers(const char *b
 	if (!seuser)
 		goto err;
 
-	if (!mls_enabled)
+	if (!mls_enabled || !mls_found)
 		goto out; /* skip any MLS level */
 
 	start = ++end;
@@ -63,11 +64,9 @@ static int process_seusers(const char *b
 		end++;
 	*end = 0;
 
-	if (*start && !isspace(*start)) {
-		level = strdup(start);
-		if (!level)
-			goto err;
-	}
+	level = strdup(start);
+	if (!level)
+		goto err;
 
 out:
 	free(newbuf);

Re: [ SEMANAGE] Resync seuser parser

[Ivan Gyurdiev]

Now I notice that the libsemanage parser will fail if mls_enabled=0, 
mls_found=1, while libselinux will accept that. Which should be fixed? 
For comparison, note genusers/checkpolicy (?)/libsemanage will all fail 
parsing users if mls_enabled=0, mls_found=1.



--
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] Resync seuser parser

[Stephen Smalley]

On Fri, 2005-11-04 at 09:51 -0500, Ivan Gyurdiev wrote:
> Now I notice that the libsemanage parser will fail if mls_enabled=0, 
> mls_found=1, while libselinux will accept that. Which should be fixed? 
> For comparison, note genusers/checkpolicy (?)/libsemanage will all fail 
> parsing users if mls_enabled=0, mls_found=1.

In that case, libselinux should at least ignore the level (i.e. leave it
NULL) even if specified, because otherwise, it will ultimately be used
in a context which will then be invalid.

-- 
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] Resync seuser parser

[Stephen Smalley]

On Fri, 2005-11-04 at 02:59 -0500, Ivan Gyurdiev wrote:
> This patch is possibly wrong...  do you want the ':MLS' skipped, or only 
> the 'MLS' (keeping the semicolon). If the second, then I need to change 
> my patch a bit (but we still need to keep the parsers synchronized).

The libselinux one presently allows omission of :MLS if MLS is disabled,
but otherwise requires at least the : if MLS is enabled.  Not sure
whether that is desirable.
  
-- 
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.