genhomedircon is broken in libsemanage

genhomedircon is broken in libsemanage

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Adding

mythtv:x:1004:1004::/var/lib/mythtv:/bin/bash

To /etc/passwd causes the labeling to get all screwed up.  This would
report an error when we used the python version of genhomedircon and not
foul up the labeling by checking if there was a label for /var/lib
already in the file_context file. (Boy do I love python.  :^))

https://bugzilla.redhat.com/show_bug.cgi?id=430195

Dan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkefQFEACgkQrlYvE4MpobOQ+ACgochR6fDi53bJva3/g7+QaM9p
DJwAoN4LZp7Z3+frZj+kekxbCEJblzbw
=0p5A
-----END PGP SIGNATURE-----

--
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: genhomedircon is broken in libsemanage

[Todd Miller]

Daniel J Walsh wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Adding
> 
> mythtv:x:1004:1004::/var/lib/mythtv:/bin/bash
> 
> To /etc/passwd causes the labeling to get all screwed up.  This would
> report an error when we used the python version of genhomedircon and
> not foul up the labeling by checking if there was a label for /var/lib
> already in the file_context file. (Boy do I love python.  :^))
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=430195
> 
> Dan

That shouldn't be hard to add to genhomedircon.c.  I'll take a look.

 - todd


--
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: genhomedircon is broken in libsemanage

[Todd C. Miller]

Daniel J Walsh wrote:
> Adding
> 
> mythtv:x:1004:1004::/var/lib/mythtv:/bin/bash
> 
> To /etc/passwd causes the labeling to get all screwed up.  This would
> report an error when we used the python version of genhomedircon and
> not foul up the labeling by checking if there was a label for /var/lib
> already in the file_context file. (Boy do I love python.  :^))
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=430195

The following patch should match the old behavior.  I didn't add the
warning message but I'm open to putting it in if people think that's
the thing to do.

 - todd

 genhomedircon.c |   97 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 89 insertions(+), 8 deletions(-)

Index: trunk/libsemanage/src/genhomedircon.c
===================================================================
--- trunk/libsemanage/src/genhomedircon.c	(revision 2771)
+++ trunk/libsemanage/src/genhomedircon.c	(working copy)
@@ -24,6 +24,8 @@
 #include <semanage/seusers_policy.h>
 #include <semanage/users_policy.h>
 #include <semanage/user_record.h>
+#include <semanage/fcontext_record.h>
+#include <semanage/fcontexts_policy.h>
 #include <sepol/context.h>
 #include <sepol/context_record.h>
 #include "semanage_store.h"
@@ -45,6 +47,7 @@
 #include <pwd.h>
 #include <errno.h>
 #include <unistd.h>
+#include <regex.h>
 
 /* paths used in get_home_dirs() */
 #define PATH_ETC_USERADD "/etc/default/useradd"
@@ -101,6 +104,11 @@
 	const char *replace_with;
 } replacement_pair_t;
 
+typedef struct {
+	const char *dir;
+	int matched;
+} fc_match_handle_t;
+
 static semanage_list_t *default_shell_list(void)
 {
 	semanage_list_t *list = NULL;
@@ -150,10 +158,70 @@
 	return list;
 }
 
+/* Helper function called via semanage_fcontext_iterate() */
+static int fcontext_matches(const semanage_fcontext_t *fcontext, void *varg)
+{
+	const char *oexpr = semanage_fcontext_get_expr(fcontext);
+	fc_match_handle_t *handp = varg;
+	struct Ustr *expr;
+	regex_t re;
+	size_t n;
+	int type, retval = -1;
+
+	/* Only match ALL or DIR */
+	type = semanage_fcontext_get_type(fcontext);
+	if (type != SEMANAGE_FCONTEXT_ALL && type != SEMANAGE_FCONTEXT_ALL)
+		return 0;
+
+	/* Convert oexpr into a Ustr and anchor it at the beginning */
+	expr = ustr_dup_cstr("^");
+	if (expr == USTR_NULL)
+		goto done;
+	ustr_ins_cstr(&expr, 1, oexpr);
+	if (expr == USTR_NULL)
+		goto done;
+	n = ustr_len(expr);
+
+	/* Strip off trailing ".+" or ".*" */
+	if (ustr_cmp_suffix_cstr_eq(expr, ".+") ||
+	    ustr_cmp_suffix_cstr_eq(expr, ".*")) {
+		if (!ustr_del_subustr(&expr, n - 1, 2))
+			goto done;
+		n -= 2;
+	}
+
+	/* Strip off trailing "(/.*)?" */
+	if (ustr_cmp_suffix_cstr_eq(expr, "(/.*)?")) {
+		if (!ustr_del_subustr(&expr, n - 5, 6))
+			goto done;
+		n -= 6;
+	}
+
+	/* Append pattern to eat up trailing slashes */
+	if (!ustr_ins_cstr(&expr, n, "/*$"))
+		goto done;
+
+	/* Check dir against expr */
+	if (regcomp(&re, ustr_cstr(expr), REG_EXTENDED) != 0)
+		goto done;
+	if (regexec(&re, handp->dir, 0, NULL, 0) == 0)
+		handp->matched = 1;
+	regfree(&re);
+
+	retval = 0;
+
+done:
+	if (expr)
+		ustr_free(expr);
+
+	return retval;
+}
+
 static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
 {
 	semanage_list_t *homedir_list = NULL;
 	semanage_list_t *shells = NULL;
+	fc_match_handle_t hand;
 	char *rbuf = NULL;
 	char *path = NULL;
 	long rbuflen;
@@ -169,21 +237,18 @@
 
 	path = semanage_findval(PATH_ETC_USERADD, "HOME", "=");
 	if (path && *path) {
-		if (semanage_list_push(&homedir_list, path)) {
-			free(path);
+		if (semanage_list_push(&homedir_list, path))
 			goto fail;
-		}
 	}
 	free(path);
 
 	path = semanage_findval(PATH_ETC_LIBUSER, "LU_HOMEDIRECTORY", "=");
 	if (path && *path) {
-		if (semanage_list_push(&homedir_list, path)) {
-			free(path);
+		if (semanage_list_push(&homedir_list, path))
 			goto fail;
-		}
 	}
 	free(path);
+	path = NULL;
 
 	if (!homedir_list) {
 		if (semanage_list_push(&homedir_list, PATH_DEFAULT_HOME)) {
@@ -211,6 +276,7 @@
 		}
 	}
 	free(path);
+	path = NULL;
 
 	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
 	if (path && *path) {
@@ -221,6 +287,7 @@
 		}
 	}
 	free(path);
+	path = NULL;
 
 	if (!minuid_set) {
 		minuid = 500;
@@ -248,13 +315,26 @@
 		}
 
 		semanage_rtrim(path, '/');
+
 		if (!semanage_list_find(homedir_list, path)) {
-			if (semanage_list_push(&homedir_list, path)) {
-				free(path);
+			/*
+			 * Now check for an existing file context that matches
+			 * so we don't label a non-homedir as a homedir.
+			 */
+			hand.dir = path;
+			hand.matched = 0;
+			if (semanage_fcontext_iterate(s->h_semanage,
+			    fcontext_matches, &hand) == STATUS_ERR)
 				goto fail;
+
+			/* NOTE: old genhomedircon printed a warning on match */
+			if (!hand.matched) {
+				if (semanage_list_push(&homedir_list, path))
+					goto fail;
 			}
 		}
 		free(path);
+		path = NULL;
 	}
 
 	if (retval && retval != ENOENT) {
@@ -272,6 +352,7 @@
       fail:
 	endpwent();
 	free(rbuf);
+	free(path);
 	semanage_list_destroy(&homedir_list);
 	semanage_list_destroy(&shells);
 	return NULL;

--
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: genhomedircon is broken in libsemanage

[Joshua Brindle]

Todd Miller wrote:
> Daniel J Walsh wrote:
>   
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Adding
>>
>> mythtv:x:1004:1004::/var/lib/mythtv:/bin/bash
>>
>> To /etc/passwd causes the labeling to get all screwed up.  This would
>> report an error when we used the python version of genhomedircon and
>> not foul up the labeling by checking if there was a label for /var/lib
>> already in the file_context file. (Boy do I love python.  :^))
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=430195
>>
>> Dan
>>     
>
> That shouldn't be hard to add to genhomedircon.c.  I'll take a look.
>   

So, Todd is about to send a patch to fix this but I want to point out 
that I'm adverse to this sort of thing. There is a minuid and a null 
shell for a reason, using something above minuid with a valid shell for 
a non-interactive user is a broken configuration and the fact that we 
have to work around it is pretty unfortunate.

That said the alternative of breaking the system labeling is pretty bad, 
its probably better to hack around the problem than leave clueless users 
with broken configurations stranded but I really wish we didn't have to 
do things like this.

*sigh*


--
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: genhomedircon is broken in libsemanage

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Joshua Brindle wrote:
> Todd Miller wrote:
>> Daniel J Walsh wrote:
>>  
>>> -----BEGIN PGP SIGNED MESSAGE-----
>>> Hash: SHA1
>>>
>>> Adding
>>>
>>> mythtv:x:1004:1004::/var/lib/mythtv:/bin/bash
>>>
>>> To /etc/passwd causes the labeling to get all screwed up.  This would
>>> report an error when we used the python version of genhomedircon and
>>> not foul up the labeling by checking if there was a label for /var/lib
>>> already in the file_context file. (Boy do I love python.  :^))
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=430195
>>>
>>> Dan
>>>     
>>
>> That shouldn't be hard to add to genhomedircon.c.  I'll take a look.
>>   
> 
> So, Todd is about to send a patch to fix this but I want to point out
> that I'm adverse to this sort of thing. There is a minuid and a null
> shell for a reason, using something above minuid with a valid shell for
> a non-interactive user is a broken configuration and the fact that we
> have to work around it is pretty unfortunate.
> 
> That said the alternative of breaking the system labeling is pretty bad,
> its probably better to hack around the problem than leave clueless users
> with broken configurations stranded but I really wish we didn't have to
> do things like this.
> 
> *sigh*
> 
I think you need to scream in the semanage that this is bad behavior,
and you can't fix the labels.  The /var/lib situation is bad, but I more
commonly see admins putting real users in /usr/local or under /var.  We
need to have a message that explains this is bad and SELinux can not
handle it.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkegiewACgkQrlYvE4MpobNWTwCfX2pMShRqXwpKfjRNwQ2pLRFr
EF8AoMOUQhevXLkLEmKH3qX48RZRv0L4
=Mi3W
-----END PGP SIGNATURE-----

--
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: genhomedircon is broken in libsemanage

[Joshua Brindle]

Daniel J Walsh wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Joshua Brindle wrote:
>   
>> Todd Miller wrote:
>>     
>>> Daniel J Walsh wrote:
>>>  
>>>       
>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>> Hash: SHA1
>>>>
>>>> Adding
>>>>
>>>> mythtv:x:1004:1004::/var/lib/mythtv:/bin/bash
>>>>
>>>> To /etc/passwd causes the labeling to get all screwed up.  This would
>>>> report an error when we used the python version of genhomedircon and
>>>> not foul up the labeling by checking if there was a label for /var/lib
>>>> already in the file_context file. (Boy do I love python.  :^))
>>>>
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=430195
>>>>
>>>> Dan
>>>>     
>>>>         
>>> That shouldn't be hard to add to genhomedircon.c.  I'll take a look.
>>>   
>>>       
>> So, Todd is about to send a patch to fix this but I want to point out
>> that I'm adverse to this sort of thing. There is a minuid and a null
>> shell for a reason, using something above minuid with a valid shell for
>> a non-interactive user is a broken configuration and the fact that we
>> have to work around it is pretty unfortunate.
>>
>> That said the alternative of breaking the system labeling is pretty bad,
>> its probably better to hack around the problem than leave clueless users
>> with broken configurations stranded but I really wish we didn't have to
>> do things like this.
>>
>> *sigh*
>>
>>     
> I think you need to scream in the semanage that this is bad behavior,
> and you can't fix the labels.  The /var/lib situation is bad, but I more
> commonly see admins putting real users in /usr/local or under /var.  We
> need to have a message that explains this is bad and SELinux can not
> handle it.
>   

Well, this is part of the configurability of Linux and thats why people 
love it, right? One solution would be to effectively get rid of 
directory search denials on commodity policies (eg., give all domains 
search on all dirs for the RH policy) then we don't have to worry about 
the "top level home" directory label at all.


--
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: genhomedircon is broken in libsemanage

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Joshua Brindle wrote:
> Daniel J Walsh wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Joshua Brindle wrote:
>>  
>>> Todd Miller wrote:
>>>    
>>>> Daniel J Walsh wrote:
>>>>  
>>>>      
>>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>>> Hash: SHA1
>>>>>
>>>>> Adding
>>>>>
>>>>> mythtv:x:1004:1004::/var/lib/mythtv:/bin/bash
>>>>>
>>>>> To /etc/passwd causes the labeling to get all screwed up.  This would
>>>>> report an error when we used the python version of genhomedircon and
>>>>> not foul up the labeling by checking if there was a label for /var/lib
>>>>> already in the file_context file. (Boy do I love python.  :^))
>>>>>
>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=430195
>>>>>
>>>>> Dan
>>>>>             
>>>> That shouldn't be hard to add to genhomedircon.c.  I'll take a look.
>>>>         
>>> So, Todd is about to send a patch to fix this but I want to point out
>>> that I'm adverse to this sort of thing. There is a minuid and a null
>>> shell for a reason, using something above minuid with a valid shell for
>>> a non-interactive user is a broken configuration and the fact that we
>>> have to work around it is pretty unfortunate.
>>>
>>> That said the alternative of breaking the system labeling is pretty bad,
>>> its probably better to hack around the problem than leave clueless users
>>> with broken configurations stranded but I really wish we didn't have to
>>> do things like this.
>>>
>>> *sigh*
>>>
>>>     
>> I think you need to scream in the semanage that this is bad behavior,
>> and you can't fix the labels.  The /var/lib situation is bad, but I more
>> commonly see admins putting real users in /usr/local or under /var.  We
>> need to have a message that explains this is bad and SELinux can not
>> handle it.
>>   
> 
> Well, this is part of the configurability of Linux and thats why people
> love it, right? One solution would be to effectively get rid of
> directory search denials on commodity policies (eg., give all domains
> search on all dirs for the RH policy) then we don't have to worry about
> the "top level home" directory label at all.
> 
It is not that simple. useradd relies on home_root_t to create
directories labeled user_home_dir_t.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkegkMIACgkQrlYvE4MpobOS5QCcDfZxBWa5jVJEgrx+80h68Fuw
ev8An1vnlQ5CYlarth6aUnClkbxS8b/M
=sy3x
-----END PGP SIGNATURE-----

--
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: genhomedircon is broken in libsemanage

[Joshua Brindle]

Daniel J Walsh wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Joshua Brindle wrote:
>   
>> Daniel J Walsh wrote:
>>     
>>> -----BEGIN PGP SIGNED MESSAGE-----
>>> Hash: SHA1
>>>
>>> Joshua Brindle wrote:
>>>  
>>>       
>>>> Todd Miller wrote:
>>>>    
>>>>         
>>>>> Daniel J Walsh wrote:
>>>>>  
>>>>>      
>>>>>           
>>>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>>>> Hash: SHA1
>>>>>>
>>>>>> Adding
>>>>>>
>>>>>> mythtv:x:1004:1004::/var/lib/mythtv:/bin/bash
>>>>>>
>>>>>> To /etc/passwd causes the labeling to get all screwed up.  This would
>>>>>> report an error when we used the python version of genhomedircon and
>>>>>> not foul up the labeling by checking if there was a label for /var/lib
>>>>>> already in the file_context file. (Boy do I love python.  :^))
>>>>>>
>>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=430195
>>>>>>
>>>>>> Dan
>>>>>>             
>>>>>>             
>>>>> That shouldn't be hard to add to genhomedircon.c.  I'll take a look.
>>>>>         
>>>>>           
>>>> So, Todd is about to send a patch to fix this but I want to point out
>>>> that I'm adverse to this sort of thing. There is a minuid and a null
>>>> shell for a reason, using something above minuid with a valid shell for
>>>> a non-interactive user is a broken configuration and the fact that we
>>>> have to work around it is pretty unfortunate.
>>>>
>>>> That said the alternative of breaking the system labeling is pretty bad,
>>>> its probably better to hack around the problem than leave clueless users
>>>> with broken configurations stranded but I really wish we didn't have to
>>>> do things like this.
>>>>
>>>> *sigh*
>>>>
>>>>     
>>>>         
>>> I think you need to scream in the semanage that this is bad behavior,
>>> and you can't fix the labels.  The /var/lib situation is bad, but I more
>>> commonly see admins putting real users in /usr/local or under /var.  We
>>> need to have a message that explains this is bad and SELinux can not
>>> handle it.
>>>   
>>>       
>> Well, this is part of the configurability of Linux and thats why people
>> love it, right? One solution would be to effectively get rid of
>> directory search denials on commodity policies (eg., give all domains
>> search on all dirs for the RH policy) then we don't have to worry about
>> the "top level home" directory label at all.
>>
>>     
> It is not that simple. useradd relies on home_root_t to create
> directories labeled user_home_dir_t.
>
>   

So I think this behavior is bogus. First of all the user being added 
might not be a user_r user and thus his home directory will be 
mislabeled between the time useradd adds the directory and it gets 
relabeled. This is a race where an unprivileged user could contaminate a 
privileged users environment. Second if its true that user directories 
commonly get moved then we have to stop relying on default transitions 
to do labeling and make useradd label things properly.



--
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: genhomedircon is broken in libsemanage

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Joshua Brindle wrote:
> Daniel J Walsh wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Joshua Brindle wrote:
>>  
>>> Daniel J Walsh wrote:
>>>    
>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>> Hash: SHA1
>>>>
>>>> Joshua Brindle wrote:
>>>>  
>>>>      
>>>>> Todd Miller wrote:
>>>>>           
>>>>>> Daniel J Walsh wrote:
>>>>>>  
>>>>>>               
>>>>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>>>>> Hash: SHA1
>>>>>>>
>>>>>>> Adding
>>>>>>>
>>>>>>> mythtv:x:1004:1004::/var/lib/mythtv:/bin/bash
>>>>>>>
>>>>>>> To /etc/passwd causes the labeling to get all screwed up.  This
>>>>>>> would
>>>>>>> report an error when we used the python version of genhomedircon and
>>>>>>> not foul up the labeling by checking if there was a label for
>>>>>>> /var/lib
>>>>>>> already in the file_context file. (Boy do I love python.  :^))
>>>>>>>
>>>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=430195
>>>>>>>
>>>>>>> Dan
>>>>>>>                         
>>>>>> That shouldn't be hard to add to genhomedircon.c.  I'll take a look.
>>>>>>                   
>>>>> So, Todd is about to send a patch to fix this but I want to point out
>>>>> that I'm adverse to this sort of thing. There is a minuid and a null
>>>>> shell for a reason, using something above minuid with a valid shell
>>>>> for
>>>>> a non-interactive user is a broken configuration and the fact that we
>>>>> have to work around it is pretty unfortunate.
>>>>>
>>>>> That said the alternative of breaking the system labeling is pretty
>>>>> bad,
>>>>> its probably better to hack around the problem than leave clueless
>>>>> users
>>>>> with broken configurations stranded but I really wish we didn't
>>>>> have to
>>>>> do things like this.
>>>>>
>>>>> *sigh*
>>>>>
>>>>>             
>>>> I think you need to scream in the semanage that this is bad behavior,
>>>> and you can't fix the labels.  The /var/lib situation is bad, but I
>>>> more
>>>> commonly see admins putting real users in /usr/local or under /var.  We
>>>> need to have a message that explains this is bad and SELinux can not
>>>> handle it.
>>>>         
>>> Well, this is part of the configurability of Linux and thats why people
>>> love it, right? One solution would be to effectively get rid of
>>> directory search denials on commodity policies (eg., give all domains
>>> search on all dirs for the RH policy) then we don't have to worry about
>>> the "top level home" directory label at all.
>>>
>>>     
>> It is not that simple. useradd relies on home_root_t to create
>> directories labeled user_home_dir_t.
>>
>>   
> 
> So I think this behavior is bogus. First of all the user being added
> might not be a user_r user and thus his home directory will be
> mislabeled between the time useradd adds the directory and it gets
> relabeled. This is a race where an unprivileged user could contaminate a
> privileged users environment. Second if its true that user directories
> commonly get moved then we have to stop relying on default transitions
> to do labeling and make useradd label things properly.
> 
> 
Well two things.  First I lied a little useradd has matchpathcon built
into it and can actually to semanage if you give the -Z qualifier.
useradd -Z staff_u -dwalsh, will label dwalsh account as staff_u in
RHEL5 and Fedora 8 and less.  It will also label the directory correctly
if you do not use -Z but have __default__ set to be staff_u.  But this
does not prevent other tools from creating the directory and getting the
labeling somewhat correct (system-config-users for example.)

In Rawhide I have removed all differences in homedir labeling.  Every
user now has a homedir labeled
user_home_dir_t/user_home_t/user_mozilla_home_t.  I believe separation
on the homedir is wrong, and I no longer support it.  It is DAC
responsibility to protect users Homedirs, and the complexity of doing
this with Type Enforcement was way too complicated, for little real
security value.  Since invariably almost all users logon to a system at
the same user type.  As we move to shared home directories, with
different login types, having a consistant labeling of the homedirs
becomes critical.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkegmMkACgkQrlYvE4MpobMwBgCeLbBzPWnrkQQbvOQpsiNSVUc7
F3IAn1+n7ZAYxdM4FsW5XEFODF6oXG6Z
=CDWW
-----END PGP SIGNATURE-----

--
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: genhomedircon is broken in libsemanage

[Todd C. Miller]

Daniel J Walsh wrote:

> I think you need to scream in the semanage that this is bad behavior,
> and you can't fix the labels.  The /var/lib situation is bad, but I
> more commonly see admins putting real users in /usr/local or under /var.
> We need to have a message that explains this is bad and SELinux can not
> handle it.

Here's a version of the patch with a warning in it (wrapped for
legibility).  Reproducing the scenario in bz 430195 we get:

# semodule -Bn
libsemanage.get_home_dirs: mythtv homedir /var/lib/mythtv or its parent
directory conflicts with a file context already specified in the policy.
This usually indicates an incorrectly defined system account.  If it is
a system account please make sure its uid is less than 500 or its login
shell is /sbin/nologin.

 - todd

Index: trunk/libsemanage/src/genhomedircon.c
===================================================================
--- trunk/libsemanage/src/genhomedircon.c	(revision 2771)
+++ trunk/libsemanage/src/genhomedircon.c	(working copy)
@@ -24,6 +24,8 @@
 #include <semanage/seusers_policy.h>
 #include <semanage/users_policy.h>
 #include <semanage/user_record.h>
+#include <semanage/fcontext_record.h>
+#include <semanage/fcontexts_policy.h>
 #include <sepol/context.h>
 #include <sepol/context_record.h>
 #include "semanage_store.h"
@@ -45,6 +47,7 @@
 #include <pwd.h>
 #include <errno.h>
 #include <unistd.h>
+#include <regex.h>
 
 /* paths used in get_home_dirs() */
 #define PATH_ETC_USERADD "/etc/default/useradd"
@@ -101,6 +104,11 @@
 	const char *replace_with;
 } replacement_pair_t;
 
+typedef struct {
+	const char *dir;
+	int matched;
+} fc_match_handle_t;
+
 static semanage_list_t *default_shell_list(void)
 {
 	semanage_list_t *list = NULL;
@@ -150,10 +158,70 @@
 	return list;
 }
 
+/* Helper function called via semanage_fcontext_iterate() */
+static int fcontext_matches(const semanage_fcontext_t *fcontext, void *varg)
+{
+	const char *oexpr = semanage_fcontext_get_expr(fcontext);
+	fc_match_handle_t *handp = varg;
+	struct Ustr *expr;
+	regex_t re;
+	size_t n;
+	int type, retval = -1;
+
+	/* Only match ALL or DIR */
+	type = semanage_fcontext_get_type(fcontext);
+	if (type != SEMANAGE_FCONTEXT_ALL && type != SEMANAGE_FCONTEXT_ALL)
+		return 0;
+
+	/* Convert oexpr into a Ustr and anchor it at the beginning */
+	expr = ustr_dup_cstr("^");
+	if (expr == USTR_NULL)
+		goto done;
+	ustr_ins_cstr(&expr, 1, oexpr);
+	if (expr == USTR_NULL)
+		goto done;
+	n = ustr_len(expr);
+
+	/* Strip off trailing ".+" or ".*" */
+	if (ustr_cmp_suffix_cstr_eq(expr, ".+") ||
+	    ustr_cmp_suffix_cstr_eq(expr, ".*")) {
+		if (!ustr_del_subustr(&expr, n - 1, 2))
+			goto done;
+		n -= 2;
+	}
+
+	/* Strip off trailing "(/.*)?" */
+	if (ustr_cmp_suffix_cstr_eq(expr, "(/.*)?")) {
+		if (!ustr_del_subustr(&expr, n - 5, 6))
+			goto done;
+		n -= 6;
+	}
+
+	/* Append pattern to eat up trailing slashes */
+	if (!ustr_ins_cstr(&expr, n, "/*$"))
+		goto done;
+
+	/* Check dir against expr */
+	if (regcomp(&re, ustr_cstr(expr), REG_EXTENDED) != 0)
+		goto done;
+	if (regexec(&re, handp->dir, 0, NULL, 0) == 0)
+		handp->matched = 1;
+	regfree(&re);
+
+	retval = 0;
+
+done:
+	if (expr)
+		ustr_free(expr);
+
+	return retval;
+}
+
 static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
 {
 	semanage_list_t *homedir_list = NULL;
 	semanage_list_t *shells = NULL;
+	fc_match_handle_t hand;
 	char *rbuf = NULL;
 	char *path = NULL;
 	long rbuflen;
@@ -169,21 +237,18 @@
 
 	path = semanage_findval(PATH_ETC_USERADD, "HOME", "=");
 	if (path && *path) {
-		if (semanage_list_push(&homedir_list, path)) {
-			free(path);
+		if (semanage_list_push(&homedir_list, path))
 			goto fail;
-		}
 	}
 	free(path);
 
 	path = semanage_findval(PATH_ETC_LIBUSER, "LU_HOMEDIRECTORY", "=");
 	if (path && *path) {
-		if (semanage_list_push(&homedir_list, path)) {
-			free(path);
+		if (semanage_list_push(&homedir_list, path))
 			goto fail;
-		}
 	}
 	free(path);
+	path = NULL;
 
 	if (!homedir_list) {
 		if (semanage_list_push(&homedir_list, PATH_DEFAULT_HOME)) {
@@ -211,6 +276,7 @@
 		}
 	}
 	free(path);
+	path = NULL;
 
 	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
 	if (path && *path) {
@@ -221,6 +287,7 @@
 		}
 	}
 	free(path);
+	path = NULL;
 
 	if (!minuid_set) {
 		minuid = 500;
@@ -248,13 +315,28 @@
 		}
 
 		semanage_rtrim(path, '/');
+
 		if (!semanage_list_find(homedir_list, path)) {
-			if (semanage_list_push(&homedir_list, path)) {
-				free(path);
+			/*
+			 * Now check for an existing file context that matches
+			 * so we don't label a non-homedir as a homedir.
+			 */
+			hand.dir = path;
+			hand.matched = 0;
+			if (semanage_fcontext_iterate(s->h_semanage,
+			    fcontext_matches, &hand) == STATUS_ERR)
 				goto fail;
+
+			/* NOTE: old genhomedircon printed a warning on match */
+			if (hand.matched) {
+				WARN(s->h_semanage, "%s homedir %s or its parent directory conflicts with a file context already specified in the policy.  This usually indicates an incorrectly defined system account.  If it is a system account please make sure its uid is less than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid);
+			} else {
+				if (semanage_list_push(&homedir_list, path))
+					goto fail;
 			}
 		}
 		free(path);
+		path = NULL;
 	}
 
 	if (retval && retval != ENOENT) {
@@ -272,6 +354,7 @@
       fail:
 	endpwent();
 	free(rbuf);
+	free(path);
 	semanage_list_destroy(&homedir_list);
 	semanage_list_destroy(&shells);
 	return NULL;

--
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: genhomedircon is broken in libsemanage

[Joshua Brindle]

Todd C. Miller wrote:
> Daniel J Walsh wrote:
> 
>> I think you need to scream in the semanage that this is bad behavior,
>> and you can't fix the labels.  The /var/lib situation is bad, but I
>> more commonly see admins putting real users in /usr/local or under
>> /var. We need to have a message that explains this is bad and
>> SELinux can not handle it.
> 
> Here's a version of the patch with a warning in it (wrapped
> for legibility).  Reproducing the scenario in bz 430195 we get:
> 
> # semodule -Bn
> libsemanage.get_home_dirs: mythtv homedir /var/lib/mythtv or
> its parent directory conflicts with a file context already specified
> in the policy. This usually indicates an incorrectly defined system
>  account. If it is a system account please make sure its uid is less
> than 500 or its login shell is /sbin/nologin.
> 
>  - todd
> 

Acked-By: Joshua Brindle <method@manicmethod.com>

> Index: trunk/libsemanage/src/genhomedircon.c
> ===================================================================
> --- trunk/libsemanage/src/genhomedircon.c	(revision 2771)
> +++ trunk/libsemanage/src/genhomedircon.c	(working copy) @@ -24,6
>  +24,8 @@ #include <semanage/seusers_policy.h>
>  #include <semanage/users_policy.h>
>  #include <semanage/user_record.h>
> +#include <semanage/fcontext_record.h>
> +#include <semanage/fcontexts_policy.h>
>  #include <sepol/context.h>
>  #include <sepol/context_record.h>
>  #include "semanage_store.h"
> @@ -45,6 +47,7 @@
>  #include <pwd.h>
>  #include <errno.h>
>  #include <unistd.h>
> +#include <regex.h>
> 
>  /* paths used in get_home_dirs() */
>  #define PATH_ETC_USERADD "/etc/default/useradd"
> @@ -101,6 +104,11 @@
>  	const char *replace_with;
>  } replacement_pair_t;
> 
> +typedef struct {
> +	const char *dir;
> +	int matched;
> +} fc_match_handle_t;
> +
>  static semanage_list_t *default_shell_list(void)  {
>  	semanage_list_t *list = NULL;
> @@ -150,10 +158,70 @@
>  	return list;
>  }
> 
> +/* Helper function called via semanage_fcontext_iterate() */ static
> int +fcontext_matches(const semanage_fcontext_t *fcontext, void
> *varg) { +	const char *oexpr =
semanage_fcontext_get_expr(fcontext);
> +	fc_match_handle_t *handp = varg;
> +	struct Ustr *expr;
> +	regex_t re;
> +	size_t n;
> +	int type, retval = -1;
> +
> +	/* Only match ALL or DIR */
> +	type = semanage_fcontext_get_type(fcontext);
> +	if (type != SEMANAGE_FCONTEXT_ALL && type !=
> SEMANAGE_FCONTEXT_ALL)
> +		return 0;
> +
> +	/* Convert oexpr into a Ustr and anchor it at the beginning */
> +	expr = ustr_dup_cstr("^"); +	if (expr == USTR_NULL)
> +		goto done;
> +	ustr_ins_cstr(&expr, 1, oexpr);
> +	if (expr == USTR_NULL)
> +		goto done;
> +	n = ustr_len(expr);
> +
> +	/* Strip off trailing ".+" or ".*" */
> +	if (ustr_cmp_suffix_cstr_eq(expr, ".+") ||
> +	    ustr_cmp_suffix_cstr_eq(expr, ".*")) {
> +		if (!ustr_del_subustr(&expr, n - 1, 2))
> +			goto done;
> +		n -= 2;
> +	}
> +
> +	/* Strip off trailing "(/.*)?" */
> +	if (ustr_cmp_suffix_cstr_eq(expr, "(/.*)?")) {
> +		if (!ustr_del_subustr(&expr, n - 5, 6))
> +			goto done;
> +		n -= 6;
> +	}
> +
> +	/* Append pattern to eat up trailing slashes */
> +	if (!ustr_ins_cstr(&expr, n, "/*$"))
> +		goto done;
> +
> +	/* Check dir against expr */
> +	if (regcomp(&re, ustr_cstr(expr), REG_EXTENDED) != 0) +
goto done;
> +	if (regexec(&re, handp->dir, 0, NULL, 0) == 0)
> +		handp->matched = 1;
> +	regfree(&re);
> +
> +	retval = 0;
> +
> +done:
> +	if (expr)
> +		ustr_free(expr);
> +
> +	return retval;
> +}
> +
>  static semanage_list_t
> *get_home_dirs(genhomedircon_settings_t * s)  {
>  	semanage_list_t *homedir_list = NULL;
>  	semanage_list_t *shells = NULL;
> +	fc_match_handle_t hand;
>  	char *rbuf = NULL;
>  	char *path = NULL;
>  	long rbuflen;
> @@ -169,21 +237,18 @@
> 
>  	path = semanage_findval(PATH_ETC_USERADD, "HOME", "=");
if (path
> && *path) { 
> -		if (semanage_list_push(&homedir_list, path)) {
> -			free(path);
> +		if (semanage_list_push(&homedir_list, path))
>  			goto fail;
> -		}
>  	}
>  	free(path);
> 
>  	path = semanage_findval(PATH_ETC_LIBUSER,
> "LU_HOMEDIRECTORY", "=");
>  	if (path && *path) {
> -		if (semanage_list_push(&homedir_list, path)) {
> -			free(path);
> +		if (semanage_list_push(&homedir_list, path))
>  			goto fail;
> -		}
>  	}
>  	free(path);
> +	path = NULL;
> 
>  	if (!homedir_list) {
>  		if (semanage_list_push(&homedir_list,
> PATH_DEFAULT_HOME)) { @@ -211,6 +276,7 @@
>  		}
>  	}
>  	free(path);
> +	path = NULL;
> 
>  	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "="); 
> if (path && *path) { @@ -221,6 +287,7 @@
>  		}
>  	}
>  	free(path);
> +	path = NULL;
> 
>  	if (!minuid_set) {
>  		minuid = 500;
> @@ -248,13 +315,28 @@
>  		}
> 
>  		semanage_rtrim(path, '/');
> +
>  		if (!semanage_list_find(homedir_list, path)) {
> -			if (semanage_list_push(&homedir_list, path)) {
> -				free(path);
> +			/*
> +			 * Now check for an existing file
> context that matches
> +			 * so we don't label a non-homedir as a homedir.
+			 */
> +			hand.dir = path;
> +			hand.matched = 0;
> +			if (semanage_fcontext_iterate(s->h_semanage,
> +			    fcontext_matches, &hand) == STATUS_ERR)
>  				goto fail;
> +
> +			/* NOTE: old genhomedircon printed a
> warning on match */
> +			if (hand.matched) {
> +				WARN(s->h_semanage, "%s homedir
> %s or its parent directory conflicts with a file context
> already specified in the policy.  This usually indicates an
> incorrectly defined system account.  If it is a system
> account please make sure its uid is less than %u or its login
> shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid);
> +			} else { +				if
> (semanage_list_push(&homedir_list, path))
> +					goto fail;
>  			}
>  		}
>  		free(path);
> +		path = NULL;
>  	}
> 
>  	if (retval && retval != ENOENT) {
> @@ -272,6 +354,7 @@
>        fail:
>  	endpwent();
>  	free(rbuf);
> +	free(path);
>  	semanage_list_destroy(&homedir_list);
>  	semanage_list_destroy(&shells);
>  	return NULL;



--
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: genhomedircon is broken in libsemanage

[James Antill]

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


 Mostly FYI, although there is one minor error dealing with a malloc()
error case.

On Thu, 2008-01-31 at 10:13 -0500, Todd C. Miller wrote:

> Index: trunk/libsemanage/src/genhomedircon.c
> ===================================================================
> --- trunk/libsemanage/src/genhomedircon.c	(revision 2771)
> +++ trunk/libsemanage/src/genhomedircon.c	(working copy)
> @@ -24,6 +24,8 @@
>  #include <semanage/seusers_policy.h>
>  #include <semanage/users_policy.h>
>  #include <semanage/user_record.h>
> +#include <semanage/fcontext_record.h>
> +#include <semanage/fcontexts_policy.h>
>  #include <sepol/context.h>
>  #include <sepol/context_record.h>
>  #include "semanage_store.h"
> @@ -45,6 +47,7 @@
>  #include <pwd.h>
>  #include <errno.h>
>  #include <unistd.h>
> +#include <regex.h>
>  
>  /* paths used in get_home_dirs() */
>  #define PATH_ETC_USERADD "/etc/default/useradd"
> @@ -101,6 +104,11 @@
>  	const char *replace_with;
>  } replacement_pair_t;
>  
> +typedef struct {
> +	const char *dir;
> +	int matched;
> +} fc_match_handle_t;
> +
>  static semanage_list_t *default_shell_list(void)
>  {
>  	semanage_list_t *list = NULL;
> @@ -150,10 +158,70 @@
>  	return list;
>  }
>  
> +/* Helper function called via semanage_fcontext_iterate() */
> +static int fcontext_matches(const semanage_fcontext_t *fcontext, void *varg)
> +{
> +	const char *oexpr = semanage_fcontext_get_expr(fcontext);
> +	fc_match_handle_t *handp = varg;
> +	struct Ustr *expr;
> +	regex_t re;
> +	size_t n;
> +	int type, retval = -1;
> +
> +	/* Only match ALL or DIR */
> +	type = semanage_fcontext_get_type(fcontext);
> +	if (type != SEMANAGE_FCONTEXT_ALL && type != SEMANAGE_FCONTEXT_ALL)
> +		return 0;
> +
> +	/* Convert oexpr into a Ustr and anchor it at the beginning */
> +	expr = ustr_dup_cstr("^");
> +	if (expr == USTR_NULL)
> +		goto done;
> +	ustr_ins_cstr(&expr, 1, oexpr);

 This works fine, but you can use:

          ustr_add_cstr(&expr, oexpr)

...which appends data, so you don't need to keep track of the offset.

> +	if (expr == USTR_NULL)

 This will never be true, you either want to test the return value or
use the "has had a memory error" flag:

          if (ustr_enomem(expr))

> +		goto done;
> +	n = ustr_len(expr);
> +
> +	/* Strip off trailing ".+" or ".*" */
> +	if (ustr_cmp_suffix_cstr_eq(expr, ".+") ||
> +	    ustr_cmp_suffix_cstr_eq(expr, ".*")) {
> +		if (!ustr_del_subustr(&expr, n - 1, 2))

 This works fine, but you can use:

                  if (!ustr_del(&expr, 2))

...which always removes the last X bytes.

> +			goto done;
> +		n -= 2;
> +	}
> +
> +	/* Strip off trailing "(/.*)?" */
> +	if (ustr_cmp_suffix_cstr_eq(expr, "(/.*)?")) {
> +		if (!ustr_del_subustr(&expr, n - 5, 6))
> +			goto done;
> +		n -= 6;
> +	}
> +
> +	/* Append pattern to eat up trailing slashes */
> +	if (!ustr_ins_cstr(&expr, n, "/*$"))
> +		goto done;
> +
> +	/* Check dir against expr */
> +	if (regcomp(&re, ustr_cstr(expr), REG_EXTENDED) != 0)
> +		goto done;
> +	if (regexec(&re, handp->dir, 0, NULL, 0) == 0)
> +		handp->matched = 1;
> +	regfree(&re);
> +
> +	retval = 0;
> +
> +done:
> +	if (expr)
> +		ustr_free(expr);

 This works fine, but:

 ustr_free(NULL);

...is guaranteed to be a noop, much like libc free(NULL).

> +
> +	return retval;
> +}


-- 
James Antill <james.antill@redhat.com>
Red Hat

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

RE: genhomedircon is broken in libsemanage

[Todd C. Miller]

James Antill wrote:
>  Mostly FYI, although there is one minor error dealing with a malloc()
> error case.

Thanks for the feedback.  I wasn't sure from the ustr API docs whether
the add/del functions applied to the end of the string.  The following
diff addresses the things you pointed out.

 - todd

 genhomedircon.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

Index: trunk/libsemanage/src/genhomedircon.c
===================================================================
--- trunk/libsemanage/src/genhomedircon.c	(revision 2774)
+++ trunk/libsemanage/src/genhomedircon.c	(working copy)
@@ -176,25 +176,24 @@
 	expr = ustr_dup_cstr("^");
 	if (expr == USTR_NULL)
 		goto done;
-	ustr_ins_cstr(&expr, 1, oexpr);
-	if (expr == USTR_NULL)
+	if (!ustr_add_cstr(&expr, oexpr))
 		goto done;
 
 	/* Strip off trailing ".+" or ".*" */
 	if (ustr_cmp_suffix_cstr_eq(expr, ".+") ||
 	    ustr_cmp_suffix_cstr_eq(expr, ".*")) {
-		if (!ustr_del_subustr(&expr, ustr_len(expr) - 1, 2))
+		if (!ustr_del(&expr, 2))
 			goto done;
 	}
 
 	/* Strip off trailing "(/.*)?" */
 	if (ustr_cmp_suffix_cstr_eq(expr, "(/.*)?")) {
-		if (!ustr_del_subustr(&expr, ustr_len(expr) - 5, 6))
+		if (!ustr_del(&expr, 6))
 			goto done;
 	}
 
 	/* Append pattern to eat up trailing slashes */
-	if (!ustr_ins_cstr(&expr, ustr_len(expr), "/*$"))
+	if (!ustr_add_cstr(&expr, "/*$"))
 		goto done;
 
 	/* Check dir against expr */
@@ -207,8 +206,7 @@
 	retval = 0;
 
 done:
-	if (expr)
-		ustr_free(expr);
+	ustr_free(expr);
 
 	return retval;
 }

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