[libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Manoj Srivastava]

From: Manoj Srivastava <srivasta@debian.org>

Some non-Debian packages (like qmail, shudder) create users not below
MIN_UID, but above MAX_UID, in /etc/login.defs (non-system users are
supposed to have uids between MIN_UID and MAX_UID.

genhomedircon.c:gethomedirs() checks pwent.pw_uid against MIN_UID in
/etc/login.defs to exclude system users from generating homedir
contexts. But unfortunately it does not check it against MAX_UID
setting from the same file.

This gets us lines like the following in the
contexts/files/file_contexts.homedirs file:

,----
|  #
|  # Home Context for user user_u
|  #
|  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
|  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
|  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
|  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
|  /var/qmail/lost\+found/.*       <<none>>
|  /var/qmail      -d      system_u:object_r:home_root_t:s0
|  /var/qmail/\.journal    <<none>>
|  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
|  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
`----

This commit adds checking uid value againt MAX_UID too.

Signed-off-by: Manoj Srivastava <srivasta@debian.org>
---
 src/genhomedircon.c |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/src/genhomedircon.c b/src/genhomedircon.c
index ce15807..a5306d7 100644
--- a/src/genhomedircon.c
+++ b/src/genhomedircon.c
@@ -219,8 +219,8 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
 	char *rbuf = NULL;
 	char *path = NULL;
 	long rbuflen;
-	uid_t temp, minuid = 0;
-	int minuid_set = 0;
+	uid_t temp, minuid = 0, maxuid = 0;
+	int minuid_set = 0, maxuid_set = 0;
 	struct passwd pwstorage, *pwbuf;
 	struct stat buf;
 	int retval;
@@ -270,6 +270,16 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
 	}
 	free(path);
 	path = NULL;
+	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
+	if (path && *path) {
+		temp = atoi(path);
+		if (!maxuid_set || temp > maxuid) {
+			maxuid = temp;
+			maxuid_set = 1;
+		}
+	}
+	free(path);
+	path = NULL;
 
 	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
 	if (path && *path) {
@@ -286,6 +296,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
 		minuid = 500;
 		minuid_set = 1;
 	}
+	if (!maxuid_set) {
+		maxuid = 60000;
+		maxuid_set = 1;
+	}
 
 	rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
 	if (rbuflen <= 0)
@@ -295,7 +309,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
 		goto fail;
 	setpwent();
 	while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
-		if (pwbuf->pw_uid < minuid)
+		if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
 			continue;
 		if (!semanage_list_find(shells, pwbuf->pw_shell))
 			continue;
@@ -322,7 +336,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
 
 			/* 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);
+			  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 greater than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid, maxuid);
 			} else {
 				if (semanage_list_push(&homedir_list, path))
 					goto fail;
-- 
1.5.6.5


--
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: [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Daniel J Walsh]

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

Manoj Srivastava wrote:
> From: Manoj Srivastava <srivasta@debian.org>
> 
> Some non-Debian packages (like qmail, shudder) create users not below
> MIN_UID, but above MAX_UID, in /etc/login.defs (non-system users are
> supposed to have uids between MIN_UID and MAX_UID.
> 
> genhomedircon.c:gethomedirs() checks pwent.pw_uid against MIN_UID in
> /etc/login.defs to exclude system users from generating homedir
> contexts. But unfortunately it does not check it against MAX_UID
> setting from the same file.
> 
> This gets us lines like the following in the
> contexts/files/file_contexts.homedirs file:
> 
> ,----
> |  #
> |  # Home Context for user user_u
> |  #
> |  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
> |  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
> |  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
> |  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
> |  /var/qmail/lost\+found/.*       <<none>>
> |  /var/qmail      -d      system_u:object_r:home_root_t:s0
> |  /var/qmail/\.journal    <<none>>
> |  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
> |  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
> `----
> 
> This commit adds checking uid value againt MAX_UID too.
> 
> Signed-off-by: Manoj Srivastava <srivasta@debian.org>
> ---
>  src/genhomedircon.c |   22 ++++++++++++++++++----
>  1 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/src/genhomedircon.c b/src/genhomedircon.c
> index ce15807..a5306d7 100644
> --- a/src/genhomedircon.c
> +++ b/src/genhomedircon.c
> @@ -219,8 +219,8 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>  	char *rbuf = NULL;
>  	char *path = NULL;
>  	long rbuflen;
> -	uid_t temp, minuid = 0;
> -	int minuid_set = 0;
> +	uid_t temp, minuid = 0, maxuid = 0;
> +	int minuid_set = 0, maxuid_set = 0;
>  	struct passwd pwstorage, *pwbuf;
>  	struct stat buf;
>  	int retval;
> @@ -270,6 +270,16 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>  	}
>  	free(path);
>  	path = NULL;
> +	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
> +	if (path && *path) {
> +		temp = atoi(path);
> +		if (!maxuid_set || temp > maxuid) {
> +			maxuid = temp;
> +			maxuid_set = 1;
> +		}
> +	}
> +	free(path);
> +	path = NULL;
>  
>  	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
>  	if (path && *path) {
> @@ -286,6 +296,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>  		minuid = 500;
>  		minuid_set = 1;
>  	}
> +	if (!maxuid_set) {
> +		maxuid = 60000;
> +		maxuid_set = 1;
> +	}
>  
>  	rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>  	if (rbuflen <= 0)
> @@ -295,7 +309,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>  		goto fail;
>  	setpwent();
>  	while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
> -		if (pwbuf->pw_uid < minuid)
> +		if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>  			continue;
>  		if (!semanage_list_find(shells, pwbuf->pw_shell))
>  			continue;
> @@ -322,7 +336,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>  
>  			/* 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);
> +			  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 greater than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid, maxuid);
>  			} else {
>  				if (semanage_list_push(&homedir_list, path))
>  					goto fail;
I think the default MAX_UID is not big enough.

Shouldn't it be MAXINT?

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

iEYEARECAAYFAklikDcACgkQrlYvE4MpobOO5QCg4zOQCJ2I3ajjf0lAOKbZpq27
F6sAoIa1MaJDR+albJlApB5N+NwpxabD
=OT2M
-----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: [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Manoj Srivastava]

On Mon, Jan 05 2009, Daniel J Walsh wrote:

> Manoj Srivastava wrote:
>> From: Manoj Srivastava <srivasta@debian.org>
>> 
>> Some non-Debian packages (like qmail, shudder) create users not below
>> MIN_UID, but above MAX_UID, in /etc/login.defs (non-system users are
>> supposed to have uids between MIN_UID and MAX_UID.
>> 
>> genhomedircon.c:gethomedirs() checks pwent.pw_uid against MIN_UID in
>> /etc/login.defs to exclude system users from generating homedir
>> contexts. But unfortunately it does not check it against MAX_UID
>> setting from the same file.
>> 
>> This gets us lines like the following in the
>> contexts/files/file_contexts.homedirs file:
>> 
>> ,----
>> |  #
>> |  # Home Context for user user_u
>> |  #
>> |  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
>> |  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
>> |  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
>> |  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
>> |  /var/qmail/lost\+found/.*       <<none>>
>> |  /var/qmail      -d      system_u:object_r:home_root_t:s0
>> |  /var/qmail/\.journal    <<none>>
>> |  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
>> |  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
>> `----
>> 
>> This commit adds checking uid value againt MAX_UID too.
>> 
>> Signed-off-by: Manoj Srivastava <srivasta@debian.org>
>> ---
>>  src/genhomedircon.c |   22 ++++++++++++++++++----
>>  1 files changed, 18 insertions(+), 4 deletions(-)
>> 
>> diff --git a/src/genhomedircon.c b/src/genhomedircon.c
>> index ce15807..a5306d7 100644
>> --- a/src/genhomedircon.c
>> +++ b/src/genhomedircon.c
>> @@ -219,8 +219,8 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>  	char *rbuf = NULL;
>>  	char *path = NULL;
>>  	long rbuflen;
>> -	uid_t temp, minuid = 0;
>> -	int minuid_set = 0;
>> +	uid_t temp, minuid = 0, maxuid = 0;
>> +	int minuid_set = 0, maxuid_set = 0;
>>  	struct passwd pwstorage, *pwbuf;
>>  	struct stat buf;
>>  	int retval;
>> @@ -270,6 +270,16 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>  	}
>>  	free(path);
>>  	path = NULL;
>> +	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
>> +	if (path && *path) {
>> +		temp = atoi(path);
>> +		if (!maxuid_set || temp > maxuid) {
>> +			maxuid = temp;
>> +			maxuid_set = 1;
>> +		}
>> +	}
>> +	free(path);
>> +	path = NULL;
>>  
>>  	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
>>  	if (path && *path) {
>> @@ -286,6 +296,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>  		minuid = 500;
>>  		minuid_set = 1;
>>  	}
>> +	if (!maxuid_set) {
>> +		maxuid = 60000;
>> +		maxuid_set = 1;
>> +	}
>>  
>>  	rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>  	if (rbuflen <= 0)
>> @@ -295,7 +309,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>  		goto fail;
>>  	setpwent();
>>  	while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>> -		if (pwbuf->pw_uid < minuid)
>> +		if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>>  			continue;
>>  		if (!semanage_list_find(shells, pwbuf->pw_shell))
>>  			continue;
>> @@ -322,7 +336,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>  
>>  			/* 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);
>> +			  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 greater than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid, maxuid);
>>  			} else {
>>  				if (semanage_list_push(&homedir_list, path))
>>  					goto fail;
> I think the default MAX_UID is not big enough.
>
> Shouldn't it be MAXINT?

        Not unless I am misunderstanding the logic here. The way I see
 it, the code below:
,----
| while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
|         if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
|                 continue;
|         if (!semanage_list_find(shells, pwbuf->pw_shell))
|                 continue;
|         if (strcmp(pwbuf->pw_dir, "/") == 0)
|                 continue;
|         if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
|                 continue;
|         if (!(path = strdup(pwbuf->pw_dir))) {
|                 break;
|         }
|    /* DO STUFF HERE */
| }
`----

        Does nothing iff: pw_uid < minuid || pw_uid > maxuid, so it
 only does things if the UID is in the range legal for non-system users;
 and the UID range for system users is UID_MIN < uid < UID_MAX.

        If you change the upper bound to max int, then we will create
 the entries for UIDs in the range above 60000 -- which is where the
 bletcherous qmail install script places the qmail uid.

        The current behaviour, but not checking the upper bound of the
 acceptable user range would be equivalent to the raising the upper
 bound to INT_MAX; and indeed, would make this patch redundant.

        From login.defs(5):
,----
|        UID_MAX (number), UID_MIN (number)
|            Range of user IDs used for the creation of regular users by
|            useradd or newusers.
`----

        Therefore I think that the right thing to do would be to check
 for both the upper and the lower bound, not just the lower bound
 check, which is all we do now.

        manoj
-- 
Manoj Srivastava <manoj.srivastava@stdc.com> <srivasta@acm.org>        
1024D/BF24424C print 4966 F272 D093 B493 410B  924B 21BA DABB BF24 424C

--
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: [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Daniel J Walsh]

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

Manoj Srivastava wrote:
> On Mon, Jan 05 2009, Daniel J Walsh wrote:
> 
>> Manoj Srivastava wrote:
>>> From: Manoj Srivastava <srivasta@debian.org>
>>>
>>> Some non-Debian packages (like qmail, shudder) create users not below
>>> MIN_UID, but above MAX_UID, in /etc/login.defs (non-system users are
>>> supposed to have uids between MIN_UID and MAX_UID.
>>>
>>> genhomedircon.c:gethomedirs() checks pwent.pw_uid against MIN_UID in
>>> /etc/login.defs to exclude system users from generating homedir
>>> contexts. But unfortunately it does not check it against MAX_UID
>>> setting from the same file.
>>>
>>> This gets us lines like the following in the
>>> contexts/files/file_contexts.homedirs file:
>>>
>>> ,----
>>> |  #
>>> |  # Home Context for user user_u
>>> |  #
>>> |  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
>>> |  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
>>> |  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
>>> |  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
>>> |  /var/qmail/lost\+found/.*       <<none>>
>>> |  /var/qmail      -d      system_u:object_r:home_root_t:s0
>>> |  /var/qmail/\.journal    <<none>>
>>> |  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
>>> |  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
>>> `----
>>>
>>> This commit adds checking uid value againt MAX_UID too.
>>>
>>> Signed-off-by: Manoj Srivastava <srivasta@debian.org>
>>> ---
>>>  src/genhomedircon.c |   22 ++++++++++++++++++----
>>>  1 files changed, 18 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/genhomedircon.c b/src/genhomedircon.c
>>> index ce15807..a5306d7 100644
>>> --- a/src/genhomedircon.c
>>> +++ b/src/genhomedircon.c
>>> @@ -219,8 +219,8 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>  	char *rbuf = NULL;
>>>  	char *path = NULL;
>>>  	long rbuflen;
>>> -	uid_t temp, minuid = 0;
>>> -	int minuid_set = 0;
>>> +	uid_t temp, minuid = 0, maxuid = 0;
>>> +	int minuid_set = 0, maxuid_set = 0;
>>>  	struct passwd pwstorage, *pwbuf;
>>>  	struct stat buf;
>>>  	int retval;
>>> @@ -270,6 +270,16 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>  	}
>>>  	free(path);
>>>  	path = NULL;
>>> +	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
>>> +	if (path && *path) {
>>> +		temp = atoi(path);
>>> +		if (!maxuid_set || temp > maxuid) {
>>> +			maxuid = temp;
>>> +			maxuid_set = 1;
>>> +		}
>>> +	}
>>> +	free(path);
>>> +	path = NULL;
>>>  
>>>  	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
>>>  	if (path && *path) {
>>> @@ -286,6 +296,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>  		minuid = 500;
>>>  		minuid_set = 1;
>>>  	}
>>> +	if (!maxuid_set) {
>>> +		maxuid = 60000;
>>> +		maxuid_set = 1;
>>> +	}
>>>  
>>>  	rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>  	if (rbuflen <= 0)
>>> @@ -295,7 +309,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>  		goto fail;
>>>  	setpwent();
>>>  	while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>>> -		if (pwbuf->pw_uid < minuid)
>>> +		if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>>>  			continue;
>>>  		if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>  			continue;
>>> @@ -322,7 +336,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>  
>>>  			/* 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);
>>> +			  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 greater than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid, maxuid);
>>>  			} else {
>>>  				if (semanage_list_push(&homedir_list, path))
>>>  					goto fail;
>> I think the default MAX_UID is not big enough.
>>
>> Shouldn't it be MAXINT?
> 
>         Not unless I am misunderstanding the logic here. The way I see
>  it, the code below:
> ,----
> | while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
> |         if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
> |                 continue;
> |         if (!semanage_list_find(shells, pwbuf->pw_shell))
> |                 continue;
> |         if (strcmp(pwbuf->pw_dir, "/") == 0)
> |                 continue;
> |         if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
> |                 continue;
> |         if (!(path = strdup(pwbuf->pw_dir))) {
> |                 break;
> |         }
> |    /* DO STUFF HERE */
> | }
> `----
> 
>         Does nothing iff: pw_uid < minuid || pw_uid > maxuid, so it
>  only does things if the UID is in the range legal for non-system users;
>  and the UID range for system users is UID_MIN < uid < UID_MAX.
> 
>         If you change the upper bound to max int, then we will create
>  the entries for UIDs in the range above 60000 -- which is where the
>  bletcherous qmail install script places the qmail uid.
> 
>         The current behaviour, but not checking the upper bound of the
>  acceptable user range would be equivalent to the raising the upper
>  bound to INT_MAX; and indeed, would make this patch redundant.
> 
>         From login.defs(5):
> ,----
> |        UID_MAX (number), UID_MIN (number)
> |            Range of user IDs used for the creation of regular users by
> |            useradd or newusers.
> `----
> 
>         Therefore I think that the right thing to do would be to check
>  for both the upper and the lower bound, not just the lower bound
>  check, which is all we do now.
> 
>         manoj
my point being, if I have a legitimate UID > 60000 for a real user and
do not define UID_MAX, My account will not work.

I do not have a problem with checking UID_MAX, just that the default
should be much higer then 60000.


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

iEYEARECAAYFAkljU/oACgkQrlYvE4MpobO3HQCeJiGm+FOc4E8Bjf9mV8bBANYV
jzIAoMDWczZmf2sfKpln1E9CrQeXFFpM
=omnl
-----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: [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Manoj Srivastava]

On Tue, Jan 06 2009, Daniel J Walsh wrote:

> Manoj Srivastava wrote:
>> On Mon, Jan 05 2009, Daniel J Walsh wrote:
>> 
>>> Manoj Srivastava wrote:
>>>> From: Manoj Srivastava <srivasta@debian.org>
>>>>
>>>> Some non-Debian packages (like qmail, shudder) create users not below
>>>> MIN_UID, but above MAX_UID, in /etc/login.defs (non-system users are
>>>> supposed to have uids between MIN_UID and MAX_UID.
>>>>
>>>> genhomedircon.c:gethomedirs() checks pwent.pw_uid against MIN_UID in
>>>> /etc/login.defs to exclude system users from generating homedir
>>>> contexts. But unfortunately it does not check it against MAX_UID
>>>> setting from the same file.
>>>>
>>>> This gets us lines like the following in the
>>>> contexts/files/file_contexts.homedirs file:
>>>>
>>>> ,----
>>>> |  #
>>>> |  # Home Context for user user_u
>>>> |  #
>>>> |  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
>>>> |  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
>>>> |  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
>>>> |  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
>>>> |  /var/qmail/lost\+found/.*       <<none>>
>>>> |  /var/qmail      -d      system_u:object_r:home_root_t:s0
>>>> |  /var/qmail/\.journal    <<none>>
>>>> |  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
>>>> |  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
>>>> `----
>>>>
>>>> This commit adds checking uid value againt MAX_UID too.
>>>>
>>>> Signed-off-by: Manoj Srivastava <srivasta@debian.org>
>>>> ---
>>>>  src/genhomedircon.c |   22 ++++++++++++++++++----
>>>>  1 files changed, 18 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/src/genhomedircon.c b/src/genhomedircon.c
>>>> index ce15807..a5306d7 100644
>>>> --- a/src/genhomedircon.c
>>>> +++ b/src/genhomedircon.c
>>>> @@ -219,8 +219,8 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>  	char *rbuf = NULL;
>>>>  	char *path = NULL;
>>>>  	long rbuflen;
>>>> -	uid_t temp, minuid = 0;
>>>> -	int minuid_set = 0;
>>>> +	uid_t temp, minuid = 0, maxuid = 0;
>>>> +	int minuid_set = 0, maxuid_set = 0;
>>>>  	struct passwd pwstorage, *pwbuf;
>>>>  	struct stat buf;
>>>>  	int retval;
>>>> @@ -270,6 +270,16 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>  	}
>>>>  	free(path);
>>>>  	path = NULL;
>>>> +	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
>>>> +	if (path && *path) {
>>>> +		temp = atoi(path);
>>>> +		if (!maxuid_set || temp > maxuid) {
>>>> +			maxuid = temp;
>>>> +			maxuid_set = 1;
>>>> +		}
>>>> +	}
>>>> +	free(path);
>>>> +	path = NULL;
>>>>  
>>>>  	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
>>>>  	if (path && *path) {
>>>> @@ -286,6 +296,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>  		minuid = 500;
>>>>  		minuid_set = 1;
>>>>  	}
>>>> +	if (!maxuid_set) {
>>>> +		maxuid = 60000;
>>>> +		maxuid_set = 1;
>>>> +	}
>>>>  
>>>>  	rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>>  	if (rbuflen <= 0)
>>>> @@ -295,7 +309,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>  		goto fail;
>>>>  	setpwent();
>>>>  	while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>>>> -		if (pwbuf->pw_uid < minuid)
>>>> +		if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>>>>  			continue;
>>>>  		if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>>  			continue;
>>>> @@ -322,7 +336,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>  
>>>>  			/* 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);
>>>> +			  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 greater than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid, maxuid);
>>>>  			} else {
>>>>  				if (semanage_list_push(&homedir_list, path))
>>>>  					goto fail;
>>> I think the default MAX_UID is not big enough.
>>>
>>> Shouldn't it be MAXINT?
>> 
>>         Not unless I am misunderstanding the logic here. The way I see
>>  it, the code below:
>> ,----
>> | while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>> |         if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>> |                 continue;
>> |         if (!semanage_list_find(shells, pwbuf->pw_shell))
>> |                 continue;
>> |         if (strcmp(pwbuf->pw_dir, "/") == 0)
>> |                 continue;
>> |         if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
>> |                 continue;
>> |         if (!(path = strdup(pwbuf->pw_dir))) {
>> |                 break;
>> |         }
>> |    /* DO STUFF HERE */
>> | }
>> `----
>> 
>>         Does nothing iff: pw_uid < minuid || pw_uid > maxuid, so it
>>  only does things if the UID is in the range legal for non-system users;
>>  and the UID range for system users is UID_MIN < uid < UID_MAX.
>> 
>>         If you change the upper bound to max int, then we will create
>>  the entries for UIDs in the range above 60000 -- which is where the
>>  bletcherous qmail install script places the qmail uid.
>> 
>>         The current behaviour, but not checking the upper bound of the
>>  acceptable user range would be equivalent to the raising the upper
>>  bound to INT_MAX; and indeed, would make this patch redundant.
>> 
>>         From login.defs(5):
>> ,----
>> |        UID_MAX (number), UID_MIN (number)
>> |            Range of user IDs used for the creation of regular users by
>> |            useradd or newusers.
>> `----
>> 
>>         Therefore I think that the right thing to do would be to check
>>  for both the upper and the lower bound, not just the lower bound
>>  check, which is all we do now.
>> 
>>         manoj
> my point being, if I have a legitimate UID > 60000 for a real user and
> do not define UID_MAX, My account will not work.
>
> I do not have a problem with checking UID_MAX, just that the default
> should be much higer then 60000.

        I just went with the Debian policy default value; 60000 is the
 upper limit of uid's on Debian (and probably most Debian derivative)
 machines, and UID's lager than that are reserved by policy for Debian
 packages to use (after discussion on the development mailing lists).

        Having said that, I have no objection to making the default max
 uid a preprocessor constant, which can be tweaked by the
 distributions. Should I send in an updated patch?

        manoj
-- 
Manoj Srivastava <manoj.srivastava@stdc.com> <srivasta@acm.org>        
1024D/BF24424C print 4966 F272 D093 B493 410B  924B 21BA DABB BF24 424C

--
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: [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Stephen Smalley]

On Tue, 2009-01-06 at 08:51 -0600, Manoj Srivastava wrote:
> On Tue, Jan 06 2009, Daniel J Walsh wrote:
> 
> > Manoj Srivastava wrote:
> >> On Mon, Jan 05 2009, Daniel J Walsh wrote:
> >> 
> >>> Manoj Srivastava wrote:
> >>>> From: Manoj Srivastava <srivasta@debian.org>
> >>>>
> >>>> Some non-Debian packages (like qmail, shudder) create users not below
> >>>> MIN_UID, but above MAX_UID, in /etc/login.defs (non-system users are
> >>>> supposed to have uids between MIN_UID and MAX_UID.
> >>>>
> >>>> genhomedircon.c:gethomedirs() checks pwent.pw_uid against MIN_UID in
> >>>> /etc/login.defs to exclude system users from generating homedir
> >>>> contexts. But unfortunately it does not check it against MAX_UID
> >>>> setting from the same file.
> >>>>
> >>>> This gets us lines like the following in the
> >>>> contexts/files/file_contexts.homedirs file:
> >>>>
> >>>> ,----
> >>>> |  #
> >>>> |  # Home Context for user user_u
> >>>> |  #
> >>>> |  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
> >>>> |  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
> >>>> |  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
> >>>> |  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
> >>>> |  /var/qmail/lost\+found/.*       <<none>>
> >>>> |  /var/qmail      -d      system_u:object_r:home_root_t:s0
> >>>> |  /var/qmail/\.journal    <<none>>
> >>>> |  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
> >>>> |  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
> >>>> `----
> >>>>
> >>>> This commit adds checking uid value againt MAX_UID too.
> >>>>
> >>>> Signed-off-by: Manoj Srivastava <srivasta@debian.org>
> >>>> ---
> >>>>  src/genhomedircon.c |   22 ++++++++++++++++++----
> >>>>  1 files changed, 18 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/src/genhomedircon.c b/src/genhomedircon.c
> >>>> index ce15807..a5306d7 100644
> >>>> --- a/src/genhomedircon.c
> >>>> +++ b/src/genhomedircon.c
> >>>> @@ -219,8 +219,8 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>  	char *rbuf = NULL;
> >>>>  	char *path = NULL;
> >>>>  	long rbuflen;
> >>>> -	uid_t temp, minuid = 0;
> >>>> -	int minuid_set = 0;
> >>>> +	uid_t temp, minuid = 0, maxuid = 0;
> >>>> +	int minuid_set = 0, maxuid_set = 0;
> >>>>  	struct passwd pwstorage, *pwbuf;
> >>>>  	struct stat buf;
> >>>>  	int retval;
> >>>> @@ -270,6 +270,16 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>  	}
> >>>>  	free(path);
> >>>>  	path = NULL;
> >>>> +	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
> >>>> +	if (path && *path) {
> >>>> +		temp = atoi(path);
> >>>> +		if (!maxuid_set || temp > maxuid) {
> >>>> +			maxuid = temp;
> >>>> +			maxuid_set = 1;
> >>>> +		}
> >>>> +	}
> >>>> +	free(path);
> >>>> +	path = NULL;
> >>>>  
> >>>>  	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
> >>>>  	if (path && *path) {
> >>>> @@ -286,6 +296,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>  		minuid = 500;
> >>>>  		minuid_set = 1;
> >>>>  	}
> >>>> +	if (!maxuid_set) {
> >>>> +		maxuid = 60000;
> >>>> +		maxuid_set = 1;
> >>>> +	}
> >>>>  
> >>>>  	rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
> >>>>  	if (rbuflen <= 0)
> >>>> @@ -295,7 +309,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>  		goto fail;
> >>>>  	setpwent();
> >>>>  	while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
> >>>> -		if (pwbuf->pw_uid < minuid)
> >>>> +		if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
> >>>>  			continue;
> >>>>  		if (!semanage_list_find(shells, pwbuf->pw_shell))
> >>>>  			continue;
> >>>> @@ -322,7 +336,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>  
> >>>>  			/* 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);
> >>>> +			  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 greater than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid, maxuid);
> >>>>  			} else {
> >>>>  				if (semanage_list_push(&homedir_list, path))
> >>>>  					goto fail;
> >>> I think the default MAX_UID is not big enough.
> >>>
> >>> Shouldn't it be MAXINT?
> >> 
> >>         Not unless I am misunderstanding the logic here. The way I see
> >>  it, the code below:
> >> ,----
> >> | while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
> >> |         if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
> >> |                 continue;
> >> |         if (!semanage_list_find(shells, pwbuf->pw_shell))
> >> |                 continue;
> >> |         if (strcmp(pwbuf->pw_dir, "/") == 0)
> >> |                 continue;
> >> |         if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
> >> |                 continue;
> >> |         if (!(path = strdup(pwbuf->pw_dir))) {
> >> |                 break;
> >> |         }
> >> |    /* DO STUFF HERE */
> >> | }
> >> `----
> >> 
> >>         Does nothing iff: pw_uid < minuid || pw_uid > maxuid, so it
> >>  only does things if the UID is in the range legal for non-system users;
> >>  and the UID range for system users is UID_MIN < uid < UID_MAX.
> >> 
> >>         If you change the upper bound to max int, then we will create
> >>  the entries for UIDs in the range above 60000 -- which is where the
> >>  bletcherous qmail install script places the qmail uid.
> >> 
> >>         The current behaviour, but not checking the upper bound of the
> >>  acceptable user range would be equivalent to the raising the upper
> >>  bound to INT_MAX; and indeed, would make this patch redundant.
> >> 
> >>         From login.defs(5):
> >> ,----
> >> |        UID_MAX (number), UID_MIN (number)
> >> |            Range of user IDs used for the creation of regular users by
> >> |            useradd or newusers.
> >> `----
> >> 
> >>         Therefore I think that the right thing to do would be to check
> >>  for both the upper and the lower bound, not just the lower bound
> >>  check, which is all we do now.
> >> 
> >>         manoj
> > my point being, if I have a legitimate UID > 60000 for a real user and
> > do not define UID_MAX, My account will not work.
> >
> > I do not have a problem with checking UID_MAX, just that the default
> > should be much higer then 60000.
> 
>         I just went with the Debian policy default value; 60000 is the
>  upper limit of uid's on Debian (and probably most Debian derivative)
>  machines, and UID's lager than that are reserved by policy for Debian
>  packages to use (after discussion on the development mailing lists).

The same appears to be true in Fedora; UID_MAX is set to 60000 by
default in /etc/login.defs there as well.

>         Having said that, I have no objection to making the default max
>  uid a preprocessor constant, which can be tweaked by the
>  distributions. Should I send in an updated patch?

-- 
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: [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Daniel J Walsh]

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

Stephen Smalley wrote:
> On Tue, 2009-01-06 at 08:51 -0600, Manoj Srivastava wrote:
>> On Tue, Jan 06 2009, Daniel J Walsh wrote:
>>
>>> Manoj Srivastava wrote:
>>>> On Mon, Jan 05 2009, Daniel J Walsh wrote:
>>>>
>>>>> Manoj Srivastava wrote:
>>>>>> From: Manoj Srivastava <srivasta@debian.org>
>>>>>>
>>>>>> Some non-Debian packages (like qmail, shudder) create users not below
>>>>>> MIN_UID, but above MAX_UID, in /etc/login.defs (non-system users are
>>>>>> supposed to have uids between MIN_UID and MAX_UID.
>>>>>>
>>>>>> genhomedircon.c:gethomedirs() checks pwent.pw_uid against MIN_UID in
>>>>>> /etc/login.defs to exclude system users from generating homedir
>>>>>> contexts. But unfortunately it does not check it against MAX_UID
>>>>>> setting from the same file.
>>>>>>
>>>>>> This gets us lines like the following in the
>>>>>> contexts/files/file_contexts.homedirs file:
>>>>>>
>>>>>> ,----
>>>>>> |  #
>>>>>> |  # Home Context for user user_u
>>>>>> |  #
>>>>>> |  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
>>>>>> |  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
>>>>>> |  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
>>>>>> |  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
>>>>>> |  /var/qmail/lost\+found/.*       <<none>>
>>>>>> |  /var/qmail      -d      system_u:object_r:home_root_t:s0
>>>>>> |  /var/qmail/\.journal    <<none>>
>>>>>> |  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
>>>>>> |  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
>>>>>> `----
>>>>>>
>>>>>> This commit adds checking uid value againt MAX_UID too.
>>>>>>
>>>>>> Signed-off-by: Manoj Srivastava <srivasta@debian.org>
>>>>>> ---
>>>>>>  src/genhomedircon.c |   22 ++++++++++++++++++----
>>>>>>  1 files changed, 18 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/src/genhomedircon.c b/src/genhomedircon.c
>>>>>> index ce15807..a5306d7 100644
>>>>>> --- a/src/genhomedircon.c
>>>>>> +++ b/src/genhomedircon.c
>>>>>> @@ -219,8 +219,8 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>  	char *rbuf = NULL;
>>>>>>  	char *path = NULL;
>>>>>>  	long rbuflen;
>>>>>> -	uid_t temp, minuid = 0;
>>>>>> -	int minuid_set = 0;
>>>>>> +	uid_t temp, minuid = 0, maxuid = 0;
>>>>>> +	int minuid_set = 0, maxuid_set = 0;
>>>>>>  	struct passwd pwstorage, *pwbuf;
>>>>>>  	struct stat buf;
>>>>>>  	int retval;
>>>>>> @@ -270,6 +270,16 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>  	}
>>>>>>  	free(path);
>>>>>>  	path = NULL;
>>>>>> +	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
>>>>>> +	if (path && *path) {
>>>>>> +		temp = atoi(path);
>>>>>> +		if (!maxuid_set || temp > maxuid) {
>>>>>> +			maxuid = temp;
>>>>>> +			maxuid_set = 1;
>>>>>> +		}
>>>>>> +	}
>>>>>> +	free(path);
>>>>>> +	path = NULL;
>>>>>>  
>>>>>>  	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
>>>>>>  	if (path && *path) {
>>>>>> @@ -286,6 +296,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>  		minuid = 500;
>>>>>>  		minuid_set = 1;
>>>>>>  	}
>>>>>> +	if (!maxuid_set) {
>>>>>> +		maxuid = 60000;
>>>>>> +		maxuid_set = 1;
>>>>>> +	}
>>>>>>  
>>>>>>  	rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>>>>  	if (rbuflen <= 0)
>>>>>> @@ -295,7 +309,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>  		goto fail;
>>>>>>  	setpwent();
>>>>>>  	while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>>>>>> -		if (pwbuf->pw_uid < minuid)
>>>>>> +		if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>>>>>>  			continue;
>>>>>>  		if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>>>>  			continue;
>>>>>> @@ -322,7 +336,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>  
>>>>>>  			/* 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);
>>>>>> +			  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 greater than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid, maxuid);
>>>>>>  			} else {
>>>>>>  				if (semanage_list_push(&homedir_list, path))
>>>>>>  					goto fail;
>>>>> I think the default MAX_UID is not big enough.
>>>>>
>>>>> Shouldn't it be MAXINT?
>>>>         Not unless I am misunderstanding the logic here. The way I see
>>>>  it, the code below:
>>>> ,----
>>>> | while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>>>> |         if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>>>> |                 continue;
>>>> |         if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>> |                 continue;
>>>> |         if (strcmp(pwbuf->pw_dir, "/") == 0)
>>>> |                 continue;
>>>> |         if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
>>>> |                 continue;
>>>> |         if (!(path = strdup(pwbuf->pw_dir))) {
>>>> |                 break;
>>>> |         }
>>>> |    /* DO STUFF HERE */
>>>> | }
>>>> `----
>>>>
>>>>         Does nothing iff: pw_uid < minuid || pw_uid > maxuid, so it
>>>>  only does things if the UID is in the range legal for non-system users;
>>>>  and the UID range for system users is UID_MIN < uid < UID_MAX.
>>>>
>>>>         If you change the upper bound to max int, then we will create
>>>>  the entries for UIDs in the range above 60000 -- which is where the
>>>>  bletcherous qmail install script places the qmail uid.
>>>>
>>>>         The current behaviour, but not checking the upper bound of the
>>>>  acceptable user range would be equivalent to the raising the upper
>>>>  bound to INT_MAX; and indeed, would make this patch redundant.
>>>>
>>>>         From login.defs(5):
>>>> ,----
>>>> |        UID_MAX (number), UID_MIN (number)
>>>> |            Range of user IDs used for the creation of regular users by
>>>> |            useradd or newusers.
>>>> `----
>>>>
>>>>         Therefore I think that the right thing to do would be to check
>>>>  for both the upper and the lower bound, not just the lower bound
>>>>  check, which is all we do now.
>>>>
>>>>         manoj
>>> my point being, if I have a legitimate UID > 60000 for a real user and
>>> do not define UID_MAX, My account will not work.
>>>
>>> I do not have a problem with checking UID_MAX, just that the default
>>> should be much higer then 60000.
>>         I just went with the Debian policy default value; 60000 is the
>>  upper limit of uid's on Debian (and probably most Debian derivative)
>>  machines, and UID's lager than that are reserved by policy for Debian
>>  packages to use (after discussion on the development mailing lists).
> 
> The same appears to be true in Fedora; UID_MAX is set to 60000 by
> default in /etc/login.defs there as well.
> 
>>         Having said that, I have no objection to making the default max
>>  uid a preprocessor constant, which can be tweaked by the
>>  distributions. Should I send in an updated patch?
> 
Talking to Nalin, he thinks this number should be ignored,  Imagine a
university with > 60000 students or a large government Department (Say
DOD),  this will cause users with UID 600001 to not work correctly.

UID_MAX is just there to tell useradd to give up when trying to find the
next available UID to add, it means nothing when you have a network of
Users.

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

iEYEARECAAYFAkljhzoACgkQrlYvE4MpobPuSACfQBlP2aRK2MVWwtGy48+nXzCJ
o4gAoJqRBIyWl4XiRGvMZc6BmsQEFNAN
=UGGE
-----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: [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Daniel J Walsh]

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

Manoj Srivastava wrote:
> On Tue, Jan 06 2009, Daniel J Walsh wrote:
> 
>> Manoj Srivastava wrote:
>>> On Mon, Jan 05 2009, Daniel J Walsh wrote:
>>>
>>>> Manoj Srivastava wrote:
>>>>> From: Manoj Srivastava <srivasta@debian.org>
>>>>>
>>>>> Some non-Debian packages (like qmail, shudder) create users not below
>>>>> MIN_UID, but above MAX_UID, in /etc/login.defs (non-system users are
>>>>> supposed to have uids between MIN_UID and MAX_UID.
>>>>>
>>>>> genhomedircon.c:gethomedirs() checks pwent.pw_uid against MIN_UID in
>>>>> /etc/login.defs to exclude system users from generating homedir
>>>>> contexts. But unfortunately it does not check it against MAX_UID
>>>>> setting from the same file.
>>>>>
>>>>> This gets us lines like the following in the
>>>>> contexts/files/file_contexts.homedirs file:
>>>>>
>>>>> ,----
>>>>> |  #
>>>>> |  # Home Context for user user_u
>>>>> |  #
>>>>> |  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
>>>>> |  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
>>>>> |  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
>>>>> |  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
>>>>> |  /var/qmail/lost\+found/.*       <<none>>
>>>>> |  /var/qmail      -d      system_u:object_r:home_root_t:s0
>>>>> |  /var/qmail/\.journal    <<none>>
>>>>> |  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
>>>>> |  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
>>>>> `----
>>>>>
>>>>> This commit adds checking uid value againt MAX_UID too.
>>>>>
>>>>> Signed-off-by: Manoj Srivastava <srivasta@debian.org>
>>>>> ---
>>>>>  src/genhomedircon.c |   22 ++++++++++++++++++----
>>>>>  1 files changed, 18 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/src/genhomedircon.c b/src/genhomedircon.c
>>>>> index ce15807..a5306d7 100644
>>>>> --- a/src/genhomedircon.c
>>>>> +++ b/src/genhomedircon.c
>>>>> @@ -219,8 +219,8 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>  	char *rbuf = NULL;
>>>>>  	char *path = NULL;
>>>>>  	long rbuflen;
>>>>> -	uid_t temp, minuid = 0;
>>>>> -	int minuid_set = 0;
>>>>> +	uid_t temp, minuid = 0, maxuid = 0;
>>>>> +	int minuid_set = 0, maxuid_set = 0;
>>>>>  	struct passwd pwstorage, *pwbuf;
>>>>>  	struct stat buf;
>>>>>  	int retval;
>>>>> @@ -270,6 +270,16 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>  	}
>>>>>  	free(path);
>>>>>  	path = NULL;
>>>>> +	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
>>>>> +	if (path && *path) {
>>>>> +		temp = atoi(path);
>>>>> +		if (!maxuid_set || temp > maxuid) {
>>>>> +			maxuid = temp;
>>>>> +			maxuid_set = 1;
>>>>> +		}
>>>>> +	}
>>>>> +	free(path);
>>>>> +	path = NULL;
>>>>>  
>>>>>  	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
>>>>>  	if (path && *path) {
>>>>> @@ -286,6 +296,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>  		minuid = 500;
>>>>>  		minuid_set = 1;
>>>>>  	}
>>>>> +	if (!maxuid_set) {
>>>>> +		maxuid = 60000;
>>>>> +		maxuid_set = 1;
>>>>> +	}
>>>>>  
>>>>>  	rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>>>  	if (rbuflen <= 0)
>>>>> @@ -295,7 +309,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>  		goto fail;
>>>>>  	setpwent();
>>>>>  	while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>>>>> -		if (pwbuf->pw_uid < minuid)
>>>>> +		if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>>>>>  			continue;
>>>>>  		if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>>>  			continue;
>>>>> @@ -322,7 +336,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>  
>>>>>  			/* 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);
>>>>> +			  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 greater than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid, maxuid);
>>>>>  			} else {
>>>>>  				if (semanage_list_push(&homedir_list, path))
>>>>>  					goto fail;
>>>> I think the default MAX_UID is not big enough.
>>>>
>>>> Shouldn't it be MAXINT?
>>>         Not unless I am misunderstanding the logic here. The way I see
>>>  it, the code below:
>>> ,----
>>> | while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>>> |         if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>>> |                 continue;
>>> |         if (!semanage_list_find(shells, pwbuf->pw_shell))
>>> |                 continue;
>>> |         if (strcmp(pwbuf->pw_dir, "/") == 0)
>>> |                 continue;
>>> |         if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
>>> |                 continue;
>>> |         if (!(path = strdup(pwbuf->pw_dir))) {
>>> |                 break;
>>> |         }
>>> |    /* DO STUFF HERE */
>>> | }
>>> `----
>>>
>>>         Does nothing iff: pw_uid < minuid || pw_uid > maxuid, so it
>>>  only does things if the UID is in the range legal for non-system users;
>>>  and the UID range for system users is UID_MIN < uid < UID_MAX.
>>>
>>>         If you change the upper bound to max int, then we will create
>>>  the entries for UIDs in the range above 60000 -- which is where the
>>>  bletcherous qmail install script places the qmail uid.
>>>
>>>         The current behaviour, but not checking the upper bound of the
>>>  acceptable user range would be equivalent to the raising the upper
>>>  bound to INT_MAX; and indeed, would make this patch redundant.
>>>
>>>         From login.defs(5):
>>> ,----
>>> |        UID_MAX (number), UID_MIN (number)
>>> |            Range of user IDs used for the creation of regular users by
>>> |            useradd or newusers.
>>> `----
>>>
>>>         Therefore I think that the right thing to do would be to check
>>>  for both the upper and the lower bound, not just the lower bound
>>>  check, which is all we do now.
>>>
>>>         manoj
>> my point being, if I have a legitimate UID > 60000 for a real user and
>> do not define UID_MAX, My account will not work.
>>
>> I do not have a problem with checking UID_MAX, just that the default
>> should be much higer then 60000.
> 
>         I just went with the Debian policy default value; 60000 is the
>  upper limit of uid's on Debian (and probably most Debian derivative)
>  machines, and UID's lager than that are reserved by policy for Debian
>  packages to use (after discussion on the development mailing lists).
> 
>         Having said that, I have no objection to making the default max
>  uid a preprocessor constant, which can be tweaked by the
>  distributions. Should I send in an updated patch?
> 
>         manoj
Well that does not really solve my problem since UID_MAX is defined in
the file as 60000, it will still stop looking there.




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

iEYEARECAAYFAklji7gACgkQrlYvE4MpobNx3QCgpt8y0aCWg1tDY70o1J+Xa1v3
vIwAoI4QyCgIm0DiwXs5mdnba9Wv7Op3
=y479
-----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: [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Stephen Smalley]

On Tue, 2009-01-06 at 11:30 -0500, Daniel J Walsh wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Stephen Smalley wrote:
> > On Tue, 2009-01-06 at 08:51 -0600, Manoj Srivastava wrote:
> >> On Tue, Jan 06 2009, Daniel J Walsh wrote:
> >>
> >>> Manoj Srivastava wrote:
> >>>> On Mon, Jan 05 2009, Daniel J Walsh wrote:
> >>>>
> >>>>> Manoj Srivastava wrote:
> >>>>>> From: Manoj Srivastava <srivasta@debian.org>
> >>>>>>
> >>>>>> Some non-Debian packages (like qmail, shudder) create users not below
> >>>>>> MIN_UID, but above MAX_UID, in /etc/login.defs (non-system users are
> >>>>>> supposed to have uids between MIN_UID and MAX_UID.
> >>>>>>
> >>>>>> genhomedircon.c:gethomedirs() checks pwent.pw_uid against MIN_UID in
> >>>>>> /etc/login.defs to exclude system users from generating homedir
> >>>>>> contexts. But unfortunately it does not check it against MAX_UID
> >>>>>> setting from the same file.
> >>>>>>
> >>>>>> This gets us lines like the following in the
> >>>>>> contexts/files/file_contexts.homedirs file:
> >>>>>>
> >>>>>> ,----
> >>>>>> |  #
> >>>>>> |  # Home Context for user user_u
> >>>>>> |  #
> >>>>>> |  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
> >>>>>> |  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
> >>>>>> |  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
> >>>>>> |  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
> >>>>>> |  /var/qmail/lost\+found/.*       <<none>>
> >>>>>> |  /var/qmail      -d      system_u:object_r:home_root_t:s0
> >>>>>> |  /var/qmail/\.journal    <<none>>
> >>>>>> |  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
> >>>>>> |  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
> >>>>>> `----
> >>>>>>
> >>>>>> This commit adds checking uid value againt MAX_UID too.
> >>>>>>
> >>>>>> Signed-off-by: Manoj Srivastava <srivasta@debian.org>
> >>>>>> ---
> >>>>>>  src/genhomedircon.c |   22 ++++++++++++++++++----
> >>>>>>  1 files changed, 18 insertions(+), 4 deletions(-)
> >>>>>>
> >>>>>> diff --git a/src/genhomedircon.c b/src/genhomedircon.c
> >>>>>> index ce15807..a5306d7 100644
> >>>>>> --- a/src/genhomedircon.c
> >>>>>> +++ b/src/genhomedircon.c
> >>>>>> @@ -219,8 +219,8 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>>>  	char *rbuf = NULL;
> >>>>>>  	char *path = NULL;
> >>>>>>  	long rbuflen;
> >>>>>> -	uid_t temp, minuid = 0;
> >>>>>> -	int minuid_set = 0;
> >>>>>> +	uid_t temp, minuid = 0, maxuid = 0;
> >>>>>> +	int minuid_set = 0, maxuid_set = 0;
> >>>>>>  	struct passwd pwstorage, *pwbuf;
> >>>>>>  	struct stat buf;
> >>>>>>  	int retval;
> >>>>>> @@ -270,6 +270,16 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>>>  	}
> >>>>>>  	free(path);
> >>>>>>  	path = NULL;
> >>>>>> +	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
> >>>>>> +	if (path && *path) {
> >>>>>> +		temp = atoi(path);
> >>>>>> +		if (!maxuid_set || temp > maxuid) {
> >>>>>> +			maxuid = temp;
> >>>>>> +			maxuid_set = 1;
> >>>>>> +		}
> >>>>>> +	}
> >>>>>> +	free(path);
> >>>>>> +	path = NULL;
> >>>>>>  
> >>>>>>  	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
> >>>>>>  	if (path && *path) {
> >>>>>> @@ -286,6 +296,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>>>  		minuid = 500;
> >>>>>>  		minuid_set = 1;
> >>>>>>  	}
> >>>>>> +	if (!maxuid_set) {
> >>>>>> +		maxuid = 60000;
> >>>>>> +		maxuid_set = 1;
> >>>>>> +	}
> >>>>>>  
> >>>>>>  	rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
> >>>>>>  	if (rbuflen <= 0)
> >>>>>> @@ -295,7 +309,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>>>  		goto fail;
> >>>>>>  	setpwent();
> >>>>>>  	while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
> >>>>>> -		if (pwbuf->pw_uid < minuid)
> >>>>>> +		if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
> >>>>>>  			continue;
> >>>>>>  		if (!semanage_list_find(shells, pwbuf->pw_shell))
> >>>>>>  			continue;
> >>>>>> @@ -322,7 +336,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>>>  
> >>>>>>  			/* 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);
> >>>>>> +			  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 greater than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid, maxuid);
> >>>>>>  			} else {
> >>>>>>  				if (semanage_list_push(&homedir_list, path))
> >>>>>>  					goto fail;
> >>>>> I think the default MAX_UID is not big enough.
> >>>>>
> >>>>> Shouldn't it be MAXINT?
> >>>>         Not unless I am misunderstanding the logic here. The way I see
> >>>>  it, the code below:
> >>>> ,----
> >>>> | while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
> >>>> |         if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
> >>>> |                 continue;
> >>>> |         if (!semanage_list_find(shells, pwbuf->pw_shell))
> >>>> |                 continue;
> >>>> |         if (strcmp(pwbuf->pw_dir, "/") == 0)
> >>>> |                 continue;
> >>>> |         if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
> >>>> |                 continue;
> >>>> |         if (!(path = strdup(pwbuf->pw_dir))) {
> >>>> |                 break;
> >>>> |         }
> >>>> |    /* DO STUFF HERE */
> >>>> | }
> >>>> `----
> >>>>
> >>>>         Does nothing iff: pw_uid < minuid || pw_uid > maxuid, so it
> >>>>  only does things if the UID is in the range legal for non-system users;
> >>>>  and the UID range for system users is UID_MIN < uid < UID_MAX.
> >>>>
> >>>>         If you change the upper bound to max int, then we will create
> >>>>  the entries for UIDs in the range above 60000 -- which is where the
> >>>>  bletcherous qmail install script places the qmail uid.
> >>>>
> >>>>         The current behaviour, but not checking the upper bound of the
> >>>>  acceptable user range would be equivalent to the raising the upper
> >>>>  bound to INT_MAX; and indeed, would make this patch redundant.
> >>>>
> >>>>         From login.defs(5):
> >>>> ,----
> >>>> |        UID_MAX (number), UID_MIN (number)
> >>>> |            Range of user IDs used for the creation of regular users by
> >>>> |            useradd or newusers.
> >>>> `----
> >>>>
> >>>>         Therefore I think that the right thing to do would be to check
> >>>>  for both the upper and the lower bound, not just the lower bound
> >>>>  check, which is all we do now.
> >>>>
> >>>>         manoj
> >>> my point being, if I have a legitimate UID > 60000 for a real user and
> >>> do not define UID_MAX, My account will not work.
> >>>
> >>> I do not have a problem with checking UID_MAX, just that the default
> >>> should be much higer then 60000.
> >>         I just went with the Debian policy default value; 60000 is the
> >>  upper limit of uid's on Debian (and probably most Debian derivative)
> >>  machines, and UID's lager than that are reserved by policy for Debian
> >>  packages to use (after discussion on the development mailing lists).
> > 
> > The same appears to be true in Fedora; UID_MAX is set to 60000 by
> > default in /etc/login.defs there as well.
> > 
> >>         Having said that, I have no objection to making the default max
> >>  uid a preprocessor constant, which can be tweaked by the
> >>  distributions. Should I send in an updated patch?
> > 
> Talking to Nalin, he thinks this number should be ignored,  Imagine a
> university with > 60000 students or a large government Department (Say
> DOD),  this will cause users with UID 600001 to not work correctly.
> 
> UID_MAX is just there to tell useradd to give up when trying to find the
> next available UID to add, it means nothing when you have a network of
> Users.

This seems similar to the discussion you raised in:
http://marc.info/?l=selinux&m=122286879230076&w=2

Unfortunately there wasn't any real follow-up at the time.

-- 
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: [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Daniel J Walsh]

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

Stephen Smalley wrote:
> On Tue, 2009-01-06 at 11:30 -0500, Daniel J Walsh wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Stephen Smalley wrote:
>>> On Tue, 2009-01-06 at 08:51 -0600, Manoj Srivastava wrote:
>>>> On Tue, Jan 06 2009, Daniel J Walsh wrote:
>>>>
>>>>> Manoj Srivastava wrote:
>>>>>> On Mon, Jan 05 2009, Daniel J Walsh wrote:
>>>>>>
>>>>>>> Manoj Srivastava wrote:
>>>>>>>> From: Manoj Srivastava <srivasta@debian.org>
>>>>>>>>
>>>>>>>> Some non-Debian packages (like qmail, shudder) create users not below
>>>>>>>> MIN_UID, but above MAX_UID, in /etc/login.defs (non-system users are
>>>>>>>> supposed to have uids between MIN_UID and MAX_UID.
>>>>>>>>
>>>>>>>> genhomedircon.c:gethomedirs() checks pwent.pw_uid against MIN_UID in
>>>>>>>> /etc/login.defs to exclude system users from generating homedir
>>>>>>>> contexts. But unfortunately it does not check it against MAX_UID
>>>>>>>> setting from the same file.
>>>>>>>>
>>>>>>>> This gets us lines like the following in the
>>>>>>>> contexts/files/file_contexts.homedirs file:
>>>>>>>>
>>>>>>>> ,----
>>>>>>>> |  #
>>>>>>>> |  # Home Context for user user_u
>>>>>>>> |  #
>>>>>>>> |  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
>>>>>>>> |  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
>>>>>>>> |  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
>>>>>>>> |  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
>>>>>>>> |  /var/qmail/lost\+found/.*       <<none>>
>>>>>>>> |  /var/qmail      -d      system_u:object_r:home_root_t:s0
>>>>>>>> |  /var/qmail/\.journal    <<none>>
>>>>>>>> |  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
>>>>>>>> |  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
>>>>>>>> `----
>>>>>>>>
>>>>>>>> This commit adds checking uid value againt MAX_UID too.
>>>>>>>>
>>>>>>>> Signed-off-by: Manoj Srivastava <srivasta@debian.org>
>>>>>>>> ---
>>>>>>>>  src/genhomedircon.c |   22 ++++++++++++++++++----
>>>>>>>>  1 files changed, 18 insertions(+), 4 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/src/genhomedircon.c b/src/genhomedircon.c
>>>>>>>> index ce15807..a5306d7 100644
>>>>>>>> --- a/src/genhomedircon.c
>>>>>>>> +++ b/src/genhomedircon.c
>>>>>>>> @@ -219,8 +219,8 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>>  	char *rbuf = NULL;
>>>>>>>>  	char *path = NULL;
>>>>>>>>  	long rbuflen;
>>>>>>>> -	uid_t temp, minuid = 0;
>>>>>>>> -	int minuid_set = 0;
>>>>>>>> +	uid_t temp, minuid = 0, maxuid = 0;
>>>>>>>> +	int minuid_set = 0, maxuid_set = 0;
>>>>>>>>  	struct passwd pwstorage, *pwbuf;
>>>>>>>>  	struct stat buf;
>>>>>>>>  	int retval;
>>>>>>>> @@ -270,6 +270,16 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>>  	}
>>>>>>>>  	free(path);
>>>>>>>>  	path = NULL;
>>>>>>>> +	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
>>>>>>>> +	if (path && *path) {
>>>>>>>> +		temp = atoi(path);
>>>>>>>> +		if (!maxuid_set || temp > maxuid) {
>>>>>>>> +			maxuid = temp;
>>>>>>>> +			maxuid_set = 1;
>>>>>>>> +		}
>>>>>>>> +	}
>>>>>>>> +	free(path);
>>>>>>>> +	path = NULL;
>>>>>>>>  
>>>>>>>>  	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
>>>>>>>>  	if (path && *path) {
>>>>>>>> @@ -286,6 +296,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>>  		minuid = 500;
>>>>>>>>  		minuid_set = 1;
>>>>>>>>  	}
>>>>>>>> +	if (!maxuid_set) {
>>>>>>>> +		maxuid = 60000;
>>>>>>>> +		maxuid_set = 1;
>>>>>>>> +	}
>>>>>>>>  
>>>>>>>>  	rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>>>>>>  	if (rbuflen <= 0)
>>>>>>>> @@ -295,7 +309,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>>  		goto fail;
>>>>>>>>  	setpwent();
>>>>>>>>  	while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>>>>>>>> -		if (pwbuf->pw_uid < minuid)
>>>>>>>> +		if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>>>>>>>>  			continue;
>>>>>>>>  		if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>>>>>>  			continue;
>>>>>>>> @@ -322,7 +336,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>>  
>>>>>>>>  			/* 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);
>>>>>>>> +			  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 greater than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid, maxuid);
>>>>>>>>  			} else {
>>>>>>>>  				if (semanage_list_push(&homedir_list, path))
>>>>>>>>  					goto fail;
>>>>>>> I think the default MAX_UID is not big enough.
>>>>>>>
>>>>>>> Shouldn't it be MAXINT?
>>>>>>         Not unless I am misunderstanding the logic here. The way I see
>>>>>>  it, the code below:
>>>>>> ,----
>>>>>> | while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>>>>>> |         if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>>>>>> |                 continue;
>>>>>> |         if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>>>> |                 continue;
>>>>>> |         if (strcmp(pwbuf->pw_dir, "/") == 0)
>>>>>> |                 continue;
>>>>>> |         if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
>>>>>> |                 continue;
>>>>>> |         if (!(path = strdup(pwbuf->pw_dir))) {
>>>>>> |                 break;
>>>>>> |         }
>>>>>> |    /* DO STUFF HERE */
>>>>>> | }
>>>>>> `----
>>>>>>
>>>>>>         Does nothing iff: pw_uid < minuid || pw_uid > maxuid, so it
>>>>>>  only does things if the UID is in the range legal for non-system users;
>>>>>>  and the UID range for system users is UID_MIN < uid < UID_MAX.
>>>>>>
>>>>>>         If you change the upper bound to max int, then we will create
>>>>>>  the entries for UIDs in the range above 60000 -- which is where the
>>>>>>  bletcherous qmail install script places the qmail uid.
>>>>>>
>>>>>>         The current behaviour, but not checking the upper bound of the
>>>>>>  acceptable user range would be equivalent to the raising the upper
>>>>>>  bound to INT_MAX; and indeed, would make this patch redundant.
>>>>>>
>>>>>>         From login.defs(5):
>>>>>> ,----
>>>>>> |        UID_MAX (number), UID_MIN (number)
>>>>>> |            Range of user IDs used for the creation of regular users by
>>>>>> |            useradd or newusers.
>>>>>> `----
>>>>>>
>>>>>>         Therefore I think that the right thing to do would be to check
>>>>>>  for both the upper and the lower bound, not just the lower bound
>>>>>>  check, which is all we do now.
>>>>>>
>>>>>>         manoj
>>>>> my point being, if I have a legitimate UID > 60000 for a real user and
>>>>> do not define UID_MAX, My account will not work.
>>>>>
>>>>> I do not have a problem with checking UID_MAX, just that the default
>>>>> should be much higer then 60000.
>>>>         I just went with the Debian policy default value; 60000 is the
>>>>  upper limit of uid's on Debian (and probably most Debian derivative)
>>>>  machines, and UID's lager than that are reserved by policy for Debian
>>>>  packages to use (after discussion on the development mailing lists).
>>> The same appears to be true in Fedora; UID_MAX is set to 60000 by
>>> default in /etc/login.defs there as well.
>>>
>>>>         Having said that, I have no objection to making the default max
>>>>  uid a preprocessor constant, which can be tweaked by the
>>>>  distributions. Should I send in an updated patch?
>> Talking to Nalin, he thinks this number should be ignored,  Imagine a
>> university with > 60000 students or a large government Department (Say
>> DOD),  this will cause users with UID 600001 to not work correctly.
>>
>> UID_MAX is just there to tell useradd to give up when trying to find the
>> next available UID to add, it means nothing when you have a network of
>> Users.
> 
> This seems similar to the discussion you raised in:
> http://marc.info/?l=selinux&m=122286879230076&w=2
> 
> Unfortunately there wasn't any real follow-up at the time.
> 

I would like to make a suggestion we add a couple of flags to
/etc/selinux/semanage.conf

# semanage searches the passwd database for parents of home directories
of "real users".  semanage uses these values to setup the default file
# context, so that files in the "real users" home directory are labeled
# correctly.
# semanage tries to identify "real users" by looking for
# users with UID greater then UID_MIN as specified in /etc/login.defs.
# UID_MIN defaults to 500.  The user must also have a shell specified in
# /etc/shells, excluding /bin/false and /sbin/nologin.  On machines with
# thousands of entries in the passwd database, semanage can take a very
# long time to run.
#
# Alternatively semanage will not search the passwd database when you
# specify HOMEPARENT value.
#
# HOMEPARENT takes as its value a colon-separated list of directories
#
# HOMEPARENT=/home:/export/home:/usr/local/home
#

# Specify HOMEEXCLUDE to tell semanage to ignore these directories even
# if they seem to be parents of "real user" accounts
#
# HOMEEXCLUDE=/var/lib


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

iEYEARECAAYFAkljrUcACgkQrlYvE4MpobP+jgCbBrnnUxD1i/5rY2WiJfD04ABl
JMoAoMVGxiq6RcMj70qnXjxvK+7fRapg
=dDaC
-----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: [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Manoj Srivastava]

On Tue, Jan 06 2009, Daniel J Walsh wrote:

> Stephen Smalley wrote:
>> On Tue, 2009-01-06 at 08:51 -0600, Manoj Srivastava wrote:
>>> On Tue, Jan 06 2009, Daniel J Walsh wrote:
>>>
>>>> Manoj Srivastava wrote:
>>>>> On Mon, Jan 05 2009, Daniel J Walsh wrote:
>>>>>
>>>>>> Manoj Srivastava wrote:
>>>>>>> From: Manoj Srivastava <srivasta@debian.org>
>>>>>>>
>>>>>>> Some non-Debian packages (like qmail, shudder) create users not below
>>>>>>> MIN_UID, but above MAX_UID, in /etc/login.defs (non-system users are
>>>>>>> supposed to have uids between MIN_UID and MAX_UID.
>>>>>>>
>>>>>>> genhomedircon.c:gethomedirs() checks pwent.pw_uid against MIN_UID in
>>>>>>> /etc/login.defs to exclude system users from generating homedir
>>>>>>> contexts. But unfortunately it does not check it against MAX_UID
>>>>>>> setting from the same file.
>>>>>>>
>>>>>>> This gets us lines like the following in the
>>>>>>> contexts/files/file_contexts.homedirs file:
>>>>>>>
>>>>>>> ,----
>>>>>>> |  #
>>>>>>> |  # Home Context for user user_u
>>>>>>> |  #
>>>>>>> |  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
>>>>>>> |  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
>>>>>>> |  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
>>>>>>> |  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
>>>>>>> |  /var/qmail/lost\+found/.*       <<none>>
>>>>>>> |  /var/qmail      -d      system_u:object_r:home_root_t:s0
>>>>>>> |  /var/qmail/\.journal    <<none>>
>>>>>>> |  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
>>>>>>> |  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
>>>>>>> `----
>>>>>>>
>>>>>>> This commit adds checking uid value againt MAX_UID too.
>>>>>>>
>>>>>>> Signed-off-by: Manoj Srivastava <srivasta@debian.org>
>>>>>>> ---
>>>>>>>  src/genhomedircon.c |   22 ++++++++++++++++++----
>>>>>>>  1 files changed, 18 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/src/genhomedircon.c b/src/genhomedircon.c
>>>>>>> index ce15807..a5306d7 100644
>>>>>>> --- a/src/genhomedircon.c
>>>>>>> +++ b/src/genhomedircon.c
>>>>>>> @@ -219,8 +219,8 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>  	char *rbuf = NULL;
>>>>>>>  	char *path = NULL;
>>>>>>>  	long rbuflen;
>>>>>>> -	uid_t temp, minuid = 0;
>>>>>>> -	int minuid_set = 0;
>>>>>>> +	uid_t temp, minuid = 0, maxuid = 0;
>>>>>>> +	int minuid_set = 0, maxuid_set = 0;
>>>>>>>  	struct passwd pwstorage, *pwbuf;
>>>>>>>  	struct stat buf;
>>>>>>>  	int retval;
>>>>>>> @@ -270,6 +270,16 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>  	}
>>>>>>>  	free(path);
>>>>>>>  	path = NULL;
>>>>>>> +	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
>>>>>>> +	if (path && *path) {
>>>>>>> +		temp = atoi(path);
>>>>>>> +		if (!maxuid_set || temp > maxuid) {
>>>>>>> +			maxuid = temp;
>>>>>>> +			maxuid_set = 1;
>>>>>>> +		}
>>>>>>> +	}
>>>>>>> +	free(path);
>>>>>>> +	path = NULL;
>>>>>>>  
>>>>>>>  	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
>>>>>>>  	if (path && *path) {
>>>>>>> @@ -286,6 +296,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>  		minuid = 500;
>>>>>>>  		minuid_set = 1;
>>>>>>>  	}
>>>>>>> +	if (!maxuid_set) {
>>>>>>> +		maxuid = 60000;
>>>>>>> +		maxuid_set = 1;
>>>>>>> +	}
>>>>>>>  
>>>>>>>  	rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>>>>>  	if (rbuflen <= 0)
>>>>>>> @@ -295,7 +309,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>  		goto fail;
>>>>>>>  	setpwent();
>>>>>>>  	while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>>>>>>> -		if (pwbuf->pw_uid < minuid)
>>>>>>> +		if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>>>>>>>  			continue;
>>>>>>>  		if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>>>>>  			continue;
>>>>>>> @@ -322,7 +336,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>  
>>>>>>>  			/* 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);
>>>>>>> +			  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 greater than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid, maxuid);
>>>>>>>  			} else {
>>>>>>>  				if (semanage_list_push(&homedir_list, path))
>>>>>>>  					goto fail;
>>>>>> I think the default MAX_UID is not big enough.
>>>>>>
>>>>>> Shouldn't it be MAXINT?
>>>>>         Not unless I am misunderstanding the logic here. The way I see
>>>>>  it, the code below:
>>>>> ,----
>>>>> | while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>>>>> |         if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>>>>> |                 continue;
>>>>> |         if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>>> |                 continue;
>>>>> |         if (strcmp(pwbuf->pw_dir, "/") == 0)
>>>>> |                 continue;
>>>>> |         if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
>>>>> |                 continue;
>>>>> |         if (!(path = strdup(pwbuf->pw_dir))) {
>>>>> |                 break;
>>>>> |         }
>>>>> |    /* DO STUFF HERE */
>>>>> | }
>>>>> `----
>>>>>
>>>>>         Does nothing iff: pw_uid < minuid || pw_uid > maxuid, so it
>>>>>  only does things if the UID is in the range legal for non-system users;
>>>>>  and the UID range for system users is UID_MIN < uid < UID_MAX.
>>>>>
>>>>>         If you change the upper bound to max int, then we will create
>>>>>  the entries for UIDs in the range above 60000 -- which is where the
>>>>>  bletcherous qmail install script places the qmail uid.
>>>>>
>>>>>         The current behaviour, but not checking the upper bound of the
>>>>>  acceptable user range would be equivalent to the raising the upper
>>>>>  bound to INT_MAX; and indeed, would make this patch redundant.
>>>>>
>>>>>         From login.defs(5):
>>>>> ,----
>>>>> |        UID_MAX (number), UID_MIN (number)
>>>>> |            Range of user IDs used for the creation of regular users by
>>>>> |            useradd or newusers.
>>>>> `----
>>>>>
>>>>>         Therefore I think that the right thing to do would be to check
>>>>>  for both the upper and the lower bound, not just the lower bound
>>>>>  check, which is all we do now.
>>>>>
>>>>>         manoj
>>>> my point being, if I have a legitimate UID > 60000 for a real user and
>>>> do not define UID_MAX, My account will not work.
>>>>
>>>> I do not have a problem with checking UID_MAX, just that the default
>>>> should be much higer then 60000.
>>>         I just went with the Debian policy default value; 60000 is the
>>>  upper limit of uid's on Debian (and probably most Debian derivative)
>>>  machines, and UID's lager than that are reserved by policy for Debian
>>>  packages to use (after discussion on the development mailing lists).
>> 
>> The same appears to be true in Fedora; UID_MAX is set to 60000 by
>> default in /etc/login.defs there as well.
>> 
>>>         Having said that, I have no objection to making the default max
>>>  uid a preprocessor constant, which can be tweaked by the
>>>  distributions. Should I send in an updated patch?
>> 
> Talking to Nalin, he thinks this number should be ignored,  Imagine a
> university with > 60000 students or a large government Department (Say
> DOD),  this will cause users with UID 600001 to not work correctly.
>
> UID_MAX is just there to tell useradd to give up when trying to find the
> next available UID to add, it means nothing when you have a network of
> Users.

        Is this not why we have /etc/login.defs in the first place?
 These installations should change UID_MAX to whatever makes
 sense for their site. Otherwise, we have a mismatch between stated
 policies (/etc/logindefs.conf) and practice, and I would much rather
 have our code follow the stated policy than not.

        I think I no not see the value with ignoring the upper bound to
 user IDs, it serves as a sanity check, for example, on some machines
 against the bugs that happened due to qmail creating a user with no
 regards to policies.

        manoj
-- 
Manoj Srivastava <manoj.srivastava@stdc.com> <srivasta@acm.org>        
1024D/BF24424C print 4966 F272 D093 B493 410B  924B 21BA DABB BF24 424C

--
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: [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Stephen Smalley]

On Tue, 2009-01-06 at 18:41 -0600, Manoj Srivastava wrote:
> On Tue, Jan 06 2009, Daniel J Walsh wrote:
> 
> > Stephen Smalley wrote:
> >> On Tue, 2009-01-06 at 08:51 -0600, Manoj Srivastava wrote:
> >>> On Tue, Jan 06 2009, Daniel J Walsh wrote:
> >>>
> >>>> Manoj Srivastava wrote:
> >>>>> On Mon, Jan 05 2009, Daniel J Walsh wrote:
> >>>>>
> >>>>>> Manoj Srivastava wrote:
> >>>>>>> From: Manoj Srivastava <srivasta@debian.org>
> >>>>>>>
> >>>>>>> Some non-Debian packages (like qmail, shudder) create users not below
> >>>>>>> MIN_UID, but above MAX_UID, in /etc/login.defs (non-system users are
> >>>>>>> supposed to have uids between MIN_UID and MAX_UID.
> >>>>>>>
> >>>>>>> genhomedircon.c:gethomedirs() checks pwent.pw_uid against MIN_UID in
> >>>>>>> /etc/login.defs to exclude system users from generating homedir
> >>>>>>> contexts. But unfortunately it does not check it against MAX_UID
> >>>>>>> setting from the same file.
> >>>>>>>
> >>>>>>> This gets us lines like the following in the
> >>>>>>> contexts/files/file_contexts.homedirs file:
> >>>>>>>
> >>>>>>> ,----
> >>>>>>> |  #
> >>>>>>> |  # Home Context for user user_u
> >>>>>>> |  #
> >>>>>>> |  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
> >>>>>>> |  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
> >>>>>>> |  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
> >>>>>>> |  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
> >>>>>>> |  /var/qmail/lost\+found/.*       <<none>>
> >>>>>>> |  /var/qmail      -d      system_u:object_r:home_root_t:s0
> >>>>>>> |  /var/qmail/\.journal    <<none>>
> >>>>>>> |  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
> >>>>>>> |  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
> >>>>>>> `----
> >>>>>>>
> >>>>>>> This commit adds checking uid value againt MAX_UID too.
> >>>>>>>
> >>>>>>> Signed-off-by: Manoj Srivastava <srivasta@debian.org>
> >>>>>>> ---
> >>>>>>>  src/genhomedircon.c |   22 ++++++++++++++++++----
> >>>>>>>  1 files changed, 18 insertions(+), 4 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/src/genhomedircon.c b/src/genhomedircon.c
> >>>>>>> index ce15807..a5306d7 100644
> >>>>>>> --- a/src/genhomedircon.c
> >>>>>>> +++ b/src/genhomedircon.c
> >>>>>>> @@ -219,8 +219,8 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>>>>  	char *rbuf = NULL;
> >>>>>>>  	char *path = NULL;
> >>>>>>>  	long rbuflen;
> >>>>>>> -	uid_t temp, minuid = 0;
> >>>>>>> -	int minuid_set = 0;
> >>>>>>> +	uid_t temp, minuid = 0, maxuid = 0;
> >>>>>>> +	int minuid_set = 0, maxuid_set = 0;
> >>>>>>>  	struct passwd pwstorage, *pwbuf;
> >>>>>>>  	struct stat buf;
> >>>>>>>  	int retval;
> >>>>>>> @@ -270,6 +270,16 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>>>>  	}
> >>>>>>>  	free(path);
> >>>>>>>  	path = NULL;
> >>>>>>> +	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
> >>>>>>> +	if (path && *path) {
> >>>>>>> +		temp = atoi(path);
> >>>>>>> +		if (!maxuid_set || temp > maxuid) {
> >>>>>>> +			maxuid = temp;
> >>>>>>> +			maxuid_set = 1;
> >>>>>>> +		}
> >>>>>>> +	}
> >>>>>>> +	free(path);
> >>>>>>> +	path = NULL;
> >>>>>>>  
> >>>>>>>  	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
> >>>>>>>  	if (path && *path) {
> >>>>>>> @@ -286,6 +296,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>>>>  		minuid = 500;
> >>>>>>>  		minuid_set = 1;
> >>>>>>>  	}
> >>>>>>> +	if (!maxuid_set) {
> >>>>>>> +		maxuid = 60000;
> >>>>>>> +		maxuid_set = 1;
> >>>>>>> +	}
> >>>>>>>  
> >>>>>>>  	rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
> >>>>>>>  	if (rbuflen <= 0)
> >>>>>>> @@ -295,7 +309,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>>>>  		goto fail;
> >>>>>>>  	setpwent();
> >>>>>>>  	while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
> >>>>>>> -		if (pwbuf->pw_uid < minuid)
> >>>>>>> +		if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
> >>>>>>>  			continue;
> >>>>>>>  		if (!semanage_list_find(shells, pwbuf->pw_shell))
> >>>>>>>  			continue;
> >>>>>>> @@ -322,7 +336,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
> >>>>>>>  
> >>>>>>>  			/* 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);
> >>>>>>> +			  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 greater than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid, maxuid);
> >>>>>>>  			} else {
> >>>>>>>  				if (semanage_list_push(&homedir_list, path))
> >>>>>>>  					goto fail;
> >>>>>> I think the default MAX_UID is not big enough.
> >>>>>>
> >>>>>> Shouldn't it be MAXINT?
> >>>>>         Not unless I am misunderstanding the logic here. The way I see
> >>>>>  it, the code below:
> >>>>> ,----
> >>>>> | while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
> >>>>> |         if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
> >>>>> |                 continue;
> >>>>> |         if (!semanage_list_find(shells, pwbuf->pw_shell))
> >>>>> |                 continue;
> >>>>> |         if (strcmp(pwbuf->pw_dir, "/") == 0)
> >>>>> |                 continue;
> >>>>> |         if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
> >>>>> |                 continue;
> >>>>> |         if (!(path = strdup(pwbuf->pw_dir))) {
> >>>>> |                 break;
> >>>>> |         }
> >>>>> |    /* DO STUFF HERE */
> >>>>> | }
> >>>>> `----
> >>>>>
> >>>>>         Does nothing iff: pw_uid < minuid || pw_uid > maxuid, so it
> >>>>>  only does things if the UID is in the range legal for non-system users;
> >>>>>  and the UID range for system users is UID_MIN < uid < UID_MAX.
> >>>>>
> >>>>>         If you change the upper bound to max int, then we will create
> >>>>>  the entries for UIDs in the range above 60000 -- which is where the
> >>>>>  bletcherous qmail install script places the qmail uid.
> >>>>>
> >>>>>         The current behaviour, but not checking the upper bound of the
> >>>>>  acceptable user range would be equivalent to the raising the upper
> >>>>>  bound to INT_MAX; and indeed, would make this patch redundant.
> >>>>>
> >>>>>         From login.defs(5):
> >>>>> ,----
> >>>>> |        UID_MAX (number), UID_MIN (number)
> >>>>> |            Range of user IDs used for the creation of regular users by
> >>>>> |            useradd or newusers.
> >>>>> `----
> >>>>>
> >>>>>         Therefore I think that the right thing to do would be to check
> >>>>>  for both the upper and the lower bound, not just the lower bound
> >>>>>  check, which is all we do now.
> >>>>>
> >>>>>         manoj
> >>>> my point being, if I have a legitimate UID > 60000 for a real user and
> >>>> do not define UID_MAX, My account will not work.
> >>>>
> >>>> I do not have a problem with checking UID_MAX, just that the default
> >>>> should be much higer then 60000.
> >>>         I just went with the Debian policy default value; 60000 is the
> >>>  upper limit of uid's on Debian (and probably most Debian derivative)
> >>>  machines, and UID's lager than that are reserved by policy for Debian
> >>>  packages to use (after discussion on the development mailing lists).
> >> 
> >> The same appears to be true in Fedora; UID_MAX is set to 60000 by
> >> default in /etc/login.defs there as well.
> >> 
> >>>         Having said that, I have no objection to making the default max
> >>>  uid a preprocessor constant, which can be tweaked by the
> >>>  distributions. Should I send in an updated patch?
> >> 
> > Talking to Nalin, he thinks this number should be ignored,  Imagine a
> > university with > 60000 students or a large government Department (Say
> > DOD),  this will cause users with UID 600001 to not work correctly.
> >
> > UID_MAX is just there to tell useradd to give up when trying to find the
> > next available UID to add, it means nothing when you have a network of
> > Users.
> 
>         Is this not why we have /etc/login.defs in the first place?
>  These installations should change UID_MAX to whatever makes
>  sense for their site. Otherwise, we have a mismatch between stated
>  policies (/etc/logindefs.conf) and practice, and I would much rather
>  have our code follow the stated policy than not.

As Dan pointed out, the UID_MAX value in login.defs is only used by
useradd, and is not even strictly enforced (useradd -u 60002 john works
just fine).  In an environment with a large number of users and a global
user database, you can certainly exceed 60000 users or you may even
happen to generate your uids in a manner that happens to put them all in
the upper part of the uid space.  There are real systems with uids >
60000 for real users, yet the login.defs UID_MAX value has not been
changed on such systems because it is irrelevant and it isn't enforced
by anything.  So this patch would change behavior of libsemanage on such
systems in an undesirable manner.  And it wouldn't help with cases like
oracle where the pseudo user is added via useradd without any specified
uid at all.

I think Dan's earlier posting gets to more of the fundamental problems
with genhomedircon's heuristics for finding home directory locations,
and we need to address his points if we want it to work in general.

> 
>         I think I no not see the value with ignoring the upper bound to
>  user IDs, it serves as a sanity check, for example, on some machines
>  against the bugs that happened due to qmail creating a user with no
>  regards to policies.

-- 
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: [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Manoj Srivastava]

On Wed, Jan 07 2009, Stephen Smalley wrote:

> On Tue, 2009-01-06 at 18:41 -0600, Manoj Srivastava wrote:
>> On Tue, Jan 06 2009, Daniel J Walsh wrote:
>> 
>> > Stephen Smalley wrote:
>> >> On Tue, 2009-01-06 at 08:51 -0600, Manoj Srivastava wrote:
>> >>> On Tue, Jan 06 2009, Daniel J Walsh wrote:
>> >>>
>> >>>> Manoj Srivastava wrote:
>> >>>>> On Mon, Jan 05 2009, Daniel J Walsh wrote:
>> >>>>>
>> >>>>>> Manoj Srivastava wrote:
>> >>>>>>> From: Manoj Srivastava <srivasta@debian.org>
>> >>>>>>>
>> >>>>>>> Some non-Debian packages (like qmail, shudder) create users not below
>> >>>>>>> MIN_UID, but above MAX_UID, in /etc/login.defs (non-system users are
>> >>>>>>> supposed to have uids between MIN_UID and MAX_UID.
>> >>>>>>>
>> >>>>>>> genhomedircon.c:gethomedirs() checks pwent.pw_uid against MIN_UID in
>> >>>>>>> /etc/login.defs to exclude system users from generating homedir
>> >>>>>>> contexts. But unfortunately it does not check it against MAX_UID
>> >>>>>>> setting from the same file.
>> >>>>>>>
>> >>>>>>> This gets us lines like the following in the
>> >>>>>>> contexts/files/file_contexts.homedirs file:
>> >>>>>>>
>> >>>>>>> ,----
>> >>>>>>> |  #
>> >>>>>>> |  # Home Context for user user_u
>> >>>>>>> |  #
>> >>>>>>> |  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
>> >>>>>>> |  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
>> >>>>>>> |  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
>> >>>>>>> |  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
>> >>>>>>> |  /var/qmail/lost\+found/.*       <<none>>
>> >>>>>>> |  /var/qmail      -d      system_u:object_r:home_root_t:s0
>> >>>>>>> |  /var/qmail/\.journal    <<none>>
>> >>>>>>> |  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
>> >>>>>>> |  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
>> >>>>>>> `----
>> >>>>>>>
>> >>>>>>> This commit adds checking uid value againt MAX_UID too.
>> >>>>>>>
>> >>>>>>> Signed-off-by: Manoj Srivastava <srivasta@debian.org>
>> >>>>>>> ---
>> >>>>>>>  src/genhomedircon.c |   22 ++++++++++++++++++----
>> >>>>>>>  1 files changed, 18 insertions(+), 4 deletions(-)
>> >>>>>>>
>> >>>>>>> diff --git a/src/genhomedircon.c b/src/genhomedircon.c
>> >>>>>>> index ce15807..a5306d7 100644
>> >>>>>>> --- a/src/genhomedircon.c
>> >>>>>>> +++ b/src/genhomedircon.c
>> >>>>>>> @@ -219,8 +219,8 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>> >>>>>>>  	char *rbuf = NULL;
>> >>>>>>>  	char *path = NULL;
>> >>>>>>>  	long rbuflen;
>> >>>>>>> -	uid_t temp, minuid = 0;
>> >>>>>>> -	int minuid_set = 0;
>> >>>>>>> +	uid_t temp, minuid = 0, maxuid = 0;
>> >>>>>>> +	int minuid_set = 0, maxuid_set = 0;
>> >>>>>>>  	struct passwd pwstorage, *pwbuf;
>> >>>>>>>  	struct stat buf;
>> >>>>>>>  	int retval;
>> >>>>>>> @@ -270,6 +270,16 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>> >>>>>>>  	}
>> >>>>>>>  	free(path);
>> >>>>>>>  	path = NULL;
>> >>>>>>> +	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
>> >>>>>>> +	if (path && *path) {
>> >>>>>>> +		temp = atoi(path);
>> >>>>>>> +		if (!maxuid_set || temp > maxuid) {
>> >>>>>>> +			maxuid = temp;
>> >>>>>>> +			maxuid_set = 1;
>> >>>>>>> +		}
>> >>>>>>> +	}
>> >>>>>>> +	free(path);
>> >>>>>>> +	path = NULL;
>> >>>>>>>  
>> >>>>>>>  	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
>> >>>>>>>  	if (path && *path) {
>> >>>>>>> @@ -286,6 +296,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>> >>>>>>>  		minuid = 500;
>> >>>>>>>  		minuid_set = 1;
>> >>>>>>>  	}
>> >>>>>>> +	if (!maxuid_set) {
>> >>>>>>> +		maxuid = 60000;
>> >>>>>>> +		maxuid_set = 1;
>> >>>>>>> +	}
>> >>>>>>>  
>> >>>>>>>  	rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>> >>>>>>>  	if (rbuflen <= 0)
>> >>>>>>> @@ -295,7 +309,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>> >>>>>>>  		goto fail;
>> >>>>>>>  	setpwent();
>> >>>>>>>  	while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>> >>>>>>> -		if (pwbuf->pw_uid < minuid)
>> >>>>>>> +		if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>> >>>>>>>  			continue;
>> >>>>>>>  		if (!semanage_list_find(shells, pwbuf->pw_shell))
>> >>>>>>>  			continue;
>> >>>>>>> @@ -322,7 +336,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>> >>>>>>>  
>> >>>>>>>  			/* 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);
>> >>>>>>> +			  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 greater than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid, maxuid);
>> >>>>>>>  			} else {
>> >>>>>>>  				if (semanage_list_push(&homedir_list, path))
>> >>>>>>>  					goto fail;
>> >>>>>> I think the default MAX_UID is not big enough.
>> >>>>>>
>> >>>>>> Shouldn't it be MAXINT?
>> >>>>>         Not unless I am misunderstanding the logic here. The way I see
>> >>>>>  it, the code below:
>> >>>>> ,----
>> >>>>> | while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>> >>>>> |         if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>> >>>>> |                 continue;
>> >>>>> |         if (!semanage_list_find(shells, pwbuf->pw_shell))
>> >>>>> |                 continue;
>> >>>>> |         if (strcmp(pwbuf->pw_dir, "/") == 0)
>> >>>>> |                 continue;
>> >>>>> |         if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
>> >>>>> |                 continue;
>> >>>>> |         if (!(path = strdup(pwbuf->pw_dir))) {
>> >>>>> |                 break;
>> >>>>> |         }
>> >>>>> |    /* DO STUFF HERE */
>> >>>>> | }
>> >>>>> `----
>> >>>>>
>> >>>>>         Does nothing iff: pw_uid < minuid || pw_uid > maxuid, so it
>> >>>>>  only does things if the UID is in the range legal for non-system users;
>> >>>>>  and the UID range for system users is UID_MIN < uid < UID_MAX.
>> >>>>>
>> >>>>>         If you change the upper bound to max int, then we will create
>> >>>>>  the entries for UIDs in the range above 60000 -- which is where the
>> >>>>>  bletcherous qmail install script places the qmail uid.
>> >>>>>
>> >>>>>         The current behaviour, but not checking the upper bound of the
>> >>>>>  acceptable user range would be equivalent to the raising the upper
>> >>>>>  bound to INT_MAX; and indeed, would make this patch redundant.
>> >>>>>
>> >>>>>         From login.defs(5):
>> >>>>> ,----
>> >>>>> |        UID_MAX (number), UID_MIN (number)
>> >>>>> |            Range of user IDs used for the creation of regular users by
>> >>>>> |            useradd or newusers.
>> >>>>> `----
>> >>>>>
>> >>>>>         Therefore I think that the right thing to do would be to check
>> >>>>>  for both the upper and the lower bound, not just the lower bound
>> >>>>>  check, which is all we do now.
>> >>>>>
>> >>>>>         manoj
>> >>>> my point being, if I have a legitimate UID > 60000 for a real user and
>> >>>> do not define UID_MAX, My account will not work.
>> >>>>
>> >>>> I do not have a problem with checking UID_MAX, just that the default
>> >>>> should be much higer then 60000.
>> >>>         I just went with the Debian policy default value; 60000 is the
>> >>>  upper limit of uid's on Debian (and probably most Debian derivative)
>> >>>  machines, and UID's lager than that are reserved by policy for Debian
>> >>>  packages to use (after discussion on the development mailing lists).
>> >> 
>> >> The same appears to be true in Fedora; UID_MAX is set to 60000 by
>> >> default in /etc/login.defs there as well.
>> >> 
>> >>>         Having said that, I have no objection to making the default max
>> >>>  uid a preprocessor constant, which can be tweaked by the
>> >>>  distributions. Should I send in an updated patch?
>> >> 
>> > Talking to Nalin, he thinks this number should be ignored,  Imagine a
>> > university with > 60000 students or a large government Department (Say
>> > DOD),  this will cause users with UID 600001 to not work correctly.
>> >
>> > UID_MAX is just there to tell useradd to give up when trying to find the
>> > next available UID to add, it means nothing when you have a network of
>> > Users.
>> 
>>         Is this not why we have /etc/login.defs in the first place?
>>  These installations should change UID_MAX to whatever makes
>>  sense for their site. Otherwise, we have a mismatch between stated
>>  policies (/etc/logindefs.conf) and practice, and I would much rather
>>  have our code follow the stated policy than not.
>
> As Dan pointed out, the UID_MAX value in login.defs is only used by
> useradd, and is not even strictly enforced (useradd -u 60002 john works
> just fine).  In an environment with a large number of users and a global
> user database, you can certainly exceed 60000 users or you may even
> happen to generate your uids in a manner that happens to put them all in
> the upper part of the uid space.  There are real systems with uids >
> 60000 for real users, yet the login.defs UID_MAX value has not been
> changed on such systems because it is irrelevant and it isn't enforced
> by anything.  So this patch would change behavior of libsemanage on such
> systems in an undesirable manner.  And it wouldn't help with cases like
> oracle where the pseudo user is added via useradd without any specified
> uid at all.
>
> I think Dan's earlier posting gets to more of the fundamental problems
> with genhomedircon's heuristics for finding home directory locations,
> and we need to address his points if we want it to work in general.

        Fair enough. In that case, I would like to point out that the
 current situation of only checking UID_MIN is causing actual problems
 right now on real user systems, and making genhomedircon to incorrectly
 guess where home directories exist.

        I'll treat this as an imperfect workaround until we fix
 semodule, and then I'll just revert the patch for Debian systems.

        manoj
-- 
Manoj Srivastava <manoj.srivastava@stdc.com> <srivasta@acm.org>        
1024D/BF24424C print 4966 F272 D093 B493 410B  924B 21BA DABB BF24 424C

--
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: [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Daniel J Walsh]

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

Manoj Srivastava wrote:
> On Wed, Jan 07 2009, Stephen Smalley wrote:
> 
>> On Tue, 2009-01-06 at 18:41 -0600, Manoj Srivastava wrote:
>>> On Tue, Jan 06 2009, Daniel J Walsh wrote:
>>>
>>>> Stephen Smalley wrote:
>>>>> On Tue, 2009-01-06 at 08:51 -0600, Manoj Srivastava wrote:
>>>>>> On Tue, Jan 06 2009, Daniel J Walsh wrote:
>>>>>>
>>>>>>> Manoj Srivastava wrote:
>>>>>>>> On Mon, Jan 05 2009, Daniel J Walsh wrote:
>>>>>>>>
>>>>>>>>> Manoj Srivastava wrote:
>>>>>>>>>> From: Manoj Srivastava <srivasta@debian.org>
>>>>>>>>>>
>>>>>>>>>> Some non-Debian packages (like qmail, shudder) create users not below
>>>>>>>>>> MIN_UID, but above MAX_UID, in /etc/login.defs (non-system users are
>>>>>>>>>> supposed to have uids between MIN_UID and MAX_UID.
>>>>>>>>>>
>>>>>>>>>> genhomedircon.c:gethomedirs() checks pwent.pw_uid against MIN_UID in
>>>>>>>>>> /etc/login.defs to exclude system users from generating homedir
>>>>>>>>>> contexts. But unfortunately it does not check it against MAX_UID
>>>>>>>>>> setting from the same file.
>>>>>>>>>>
>>>>>>>>>> This gets us lines like the following in the
>>>>>>>>>> contexts/files/file_contexts.homedirs file:
>>>>>>>>>>
>>>>>>>>>> ,----
>>>>>>>>>> |  #
>>>>>>>>>> |  # Home Context for user user_u
>>>>>>>>>> |  #
>>>>>>>>>> |  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
>>>>>>>>>> |  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
>>>>>>>>>> |  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
>>>>>>>>>> |  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
>>>>>>>>>> |  /var/qmail/lost\+found/.*       <<none>>
>>>>>>>>>> |  /var/qmail      -d      system_u:object_r:home_root_t:s0
>>>>>>>>>> |  /var/qmail/\.journal    <<none>>
>>>>>>>>>> |  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
>>>>>>>>>> |  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
>>>>>>>>>> `----
>>>>>>>>>>
>>>>>>>>>> This commit adds checking uid value againt MAX_UID too.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Manoj Srivastava <srivasta@debian.org>
>>>>>>>>>> ---
>>>>>>>>>>  src/genhomedircon.c |   22 ++++++++++++++++++----
>>>>>>>>>>  1 files changed, 18 insertions(+), 4 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/src/genhomedircon.c b/src/genhomedircon.c
>>>>>>>>>> index ce15807..a5306d7 100644
>>>>>>>>>> --- a/src/genhomedircon.c
>>>>>>>>>> +++ b/src/genhomedircon.c
>>>>>>>>>> @@ -219,8 +219,8 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>>>>  	char *rbuf = NULL;
>>>>>>>>>>  	char *path = NULL;
>>>>>>>>>>  	long rbuflen;
>>>>>>>>>> -	uid_t temp, minuid = 0;
>>>>>>>>>> -	int minuid_set = 0;
>>>>>>>>>> +	uid_t temp, minuid = 0, maxuid = 0;
>>>>>>>>>> +	int minuid_set = 0, maxuid_set = 0;
>>>>>>>>>>  	struct passwd pwstorage, *pwbuf;
>>>>>>>>>>  	struct stat buf;
>>>>>>>>>>  	int retval;
>>>>>>>>>> @@ -270,6 +270,16 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>>>>  	}
>>>>>>>>>>  	free(path);
>>>>>>>>>>  	path = NULL;
>>>>>>>>>> +	path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL);
>>>>>>>>>> +	if (path && *path) {
>>>>>>>>>> +		temp = atoi(path);
>>>>>>>>>> +		if (!maxuid_set || temp > maxuid) {
>>>>>>>>>> +			maxuid = temp;
>>>>>>>>>> +			maxuid_set = 1;
>>>>>>>>>> +		}
>>>>>>>>>> +	}
>>>>>>>>>> +	free(path);
>>>>>>>>>> +	path = NULL;
>>>>>>>>>>  
>>>>>>>>>>  	path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=");
>>>>>>>>>>  	if (path && *path) {
>>>>>>>>>> @@ -286,6 +296,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>>>>  		minuid = 500;
>>>>>>>>>>  		minuid_set = 1;
>>>>>>>>>>  	}
>>>>>>>>>> +	if (!maxuid_set) {
>>>>>>>>>> +		maxuid = 60000;
>>>>>>>>>> +		maxuid_set = 1;
>>>>>>>>>> +	}
>>>>>>>>>>  
>>>>>>>>>>  	rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>>>>>>>>  	if (rbuflen <= 0)
>>>>>>>>>> @@ -295,7 +309,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>>>>  		goto fail;
>>>>>>>>>>  	setpwent();
>>>>>>>>>>  	while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>>>>>>>>>> -		if (pwbuf->pw_uid < minuid)
>>>>>>>>>> +		if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>>>>>>>>>>  			continue;
>>>>>>>>>>  		if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>>>>>>>>  			continue;
>>>>>>>>>> @@ -322,7 +336,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)
>>>>>>>>>>  
>>>>>>>>>>  			/* 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);
>>>>>>>>>> +			  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 greater than %u or its login shell is /sbin/nologin.", pwbuf->pw_name, pwbuf->pw_dir, minuid, maxuid);
>>>>>>>>>>  			} else {
>>>>>>>>>>  				if (semanage_list_push(&homedir_list, path))
>>>>>>>>>>  					goto fail;
>>>>>>>>> I think the default MAX_UID is not big enough.
>>>>>>>>>
>>>>>>>>> Shouldn't it be MAXINT?
>>>>>>>>         Not unless I am misunderstanding the logic here. The way I see
>>>>>>>>  it, the code below:
>>>>>>>> ,----
>>>>>>>> | while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, &pwbuf)) == 0) {
>>>>>>>> |         if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
>>>>>>>> |                 continue;
>>>>>>>> |         if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>>>>>> |                 continue;
>>>>>>>> |         if (strcmp(pwbuf->pw_dir, "/") == 0)
>>>>>>>> |                 continue;
>>>>>>>> |         if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
>>>>>>>> |                 continue;
>>>>>>>> |         if (!(path = strdup(pwbuf->pw_dir))) {
>>>>>>>> |                 break;
>>>>>>>> |         }
>>>>>>>> |    /* DO STUFF HERE */
>>>>>>>> | }
>>>>>>>> `----
>>>>>>>>
>>>>>>>>         Does nothing iff: pw_uid < minuid || pw_uid > maxuid, so it
>>>>>>>>  only does things if the UID is in the range legal for non-system users;
>>>>>>>>  and the UID range for system users is UID_MIN < uid < UID_MAX.
>>>>>>>>
>>>>>>>>         If you change the upper bound to max int, then we will create
>>>>>>>>  the entries for UIDs in the range above 60000 -- which is where the
>>>>>>>>  bletcherous qmail install script places the qmail uid.
>>>>>>>>
>>>>>>>>         The current behaviour, but not checking the upper bound of the
>>>>>>>>  acceptable user range would be equivalent to the raising the upper
>>>>>>>>  bound to INT_MAX; and indeed, would make this patch redundant.
>>>>>>>>
>>>>>>>>         From login.defs(5):
>>>>>>>> ,----
>>>>>>>> |        UID_MAX (number), UID_MIN (number)
>>>>>>>> |            Range of user IDs used for the creation of regular users by
>>>>>>>> |            useradd or newusers.
>>>>>>>> `----
>>>>>>>>
>>>>>>>>         Therefore I think that the right thing to do would be to check
>>>>>>>>  for both the upper and the lower bound, not just the lower bound
>>>>>>>>  check, which is all we do now.
>>>>>>>>
>>>>>>>>         manoj
>>>>>>> my point being, if I have a legitimate UID > 60000 for a real user and
>>>>>>> do not define UID_MAX, My account will not work.
>>>>>>>
>>>>>>> I do not have a problem with checking UID_MAX, just that the default
>>>>>>> should be much higer then 60000.
>>>>>>         I just went with the Debian policy default value; 60000 is the
>>>>>>  upper limit of uid's on Debian (and probably most Debian derivative)
>>>>>>  machines, and UID's lager than that are reserved by policy for Debian
>>>>>>  packages to use (after discussion on the development mailing lists).
>>>>> The same appears to be true in Fedora; UID_MAX is set to 60000 by
>>>>> default in /etc/login.defs there as well.
>>>>>
>>>>>>         Having said that, I have no objection to making the default max
>>>>>>  uid a preprocessor constant, which can be tweaked by the
>>>>>>  distributions. Should I send in an updated patch?
>>>> Talking to Nalin, he thinks this number should be ignored,  Imagine a
>>>> university with > 60000 students or a large government Department (Say
>>>> DOD),  this will cause users with UID 600001 to not work correctly.
>>>>
>>>> UID_MAX is just there to tell useradd to give up when trying to find the
>>>> next available UID to add, it means nothing when you have a network of
>>>> Users.
>>>         Is this not why we have /etc/login.defs in the first place?
>>>  These installations should change UID_MAX to whatever makes
>>>  sense for their site. Otherwise, we have a mismatch between stated
>>>  policies (/etc/logindefs.conf) and practice, and I would much rather
>>>  have our code follow the stated policy than not.
>> As Dan pointed out, the UID_MAX value in login.defs is only used by
>> useradd, and is not even strictly enforced (useradd -u 60002 john works
>> just fine).  In an environment with a large number of users and a global
>> user database, you can certainly exceed 60000 users or you may even
>> happen to generate your uids in a manner that happens to put them all in
>> the upper part of the uid space.  There are real systems with uids >
>> 60000 for real users, yet the login.defs UID_MAX value has not been
>> changed on such systems because it is irrelevant and it isn't enforced
>> by anything.  So this patch would change behavior of libsemanage on such
>> systems in an undesirable manner.  And it wouldn't help with cases like
>> oracle where the pseudo user is added via useradd without any specified
>> uid at all.
>>
>> I think Dan's earlier posting gets to more of the fundamental problems
>> with genhomedircon's heuristics for finding home directory locations,
>> and we need to address his points if we want it to work in general.
> 
>         Fair enough. In that case, I would like to point out that the
>  current situation of only checking UID_MIN is causing actual problems
>  right now on real user systems, and making genhomedircon to incorrectly
>  guess where home directories exist.
> 
>         I'll treat this as an imperfect workaround until we fix
>  semodule, and then I'll just revert the patch for Debian systems.
> 
>         manoj
What does the passwd entry that it is getting fooled by look like?  Does
the account really need a real shell?  IE Do people actually login to
the home directory?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkllElgACgkQrlYvE4MpobMD7wCg6fsXreti1IPpOW5rGab6IPl7
+KoAn3NUZUWxtyMnrUgXInLTsjiptglo
=lsLC
-----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: [DSE-Dev] [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Manoj Srivastava]

Hi,

 [Trimming the patch and early discussion]

On Wed, Jan 07 2009, Daniel J Walsh wrote:
> Manoj Srivastava wrote:
>> On Wed, Jan 07 2009, Stephen Smalley wrote:
>>> As Dan pointed out, the UID_MAX value in login.defs is only used by
>>> useradd, and is not even strictly enforced (useradd -u 60002 john works
>>> just fine).  In an environment with a large number of users and a global
>>> user database, you can certainly exceed 60000 users or you may even
>>> happen to generate your uids in a manner that happens to put them all in
>>> the upper part of the uid space.  There are real systems with uids >
>>> 60000 for real users, yet the login.defs UID_MAX value has not been
>>> changed on such systems because it is irrelevant and it isn't enforced
>>> by anything.  So this patch would change behavior of libsemanage on such
>>> systems in an undesirable manner.  And it wouldn't help with cases like
>>> oracle where the pseudo user is added via useradd without any specified
>>> uid at all.

>>> I think Dan's earlier posting gets to more of the fundamental problems
>>> with genhomedircon's heuristics for finding home directory locations,
>>> and we need to address his points if we want it to work in general.

>>         Fair enough. In that case, I would like to point out that the
>>  current situation of only checking UID_MIN is causing actual problems
>>  right now on real user systems, and making genhomedircon to incorrectly
>>  guess where home directories exist.

>>         I'll treat this as an imperfect workaround until we fix
>>  semodule, and then I'll just revert the patch for Debian systems.

> What does the passwd entry that it is getting fooled by look like?  Does
> the account really need a real shell?  IE Do people actually login to
> the home directory?

        I do not have passwd data from the machine in question, though I
 can ask. What I do have are the results of fixfiles relabel / :

,----[ file contexts in /var ]
|  drwxr-xr-x 15 root root  system_u:object_r:home_root_t:s0    4096 Dec 29 13:35 .
|  drwxr-xr-x 21 root root  system_u:object_r:root_t:s0         4096 Dec 29 14:21 ..
|  drwxr-xr-x  2 root root  user_u:object_r:user_home_dir_t:s0  4096 May  7  2008 backups
|  drwxr-xr-x  7 root root  user_u:object_r:user_home_dir_t:s0  4096 Dec 29 14:17 cache
|  drwxr-xr-x 25 root root  user_u:object_r:user_home_dir_t:s0  4096 Dec 29 14:17 lib
|  drwxrwsr-x  2 root staff user_u:object_r:user_home_dir_t:s0  4096 Mar 11  2008 local
|  drwxrwxrwt  2 root root  user_u:object_r:user_home_dir_t:s0  4096 Dec 29 18:14 lock
|  drwxr-xr-x  6 root root  system_u:object_r:var_log_t:s0      4096 Dec 29 18:19 log
|  drwx------  2 root root  system_u:object_r:lost_found_t:s0  16384 May  5  2008 lost+found
|  drwxrwsr-x  2 root mail  user_u:object_r:user_home_dir_t:s0  4096 May  5  2008 mail
|  drwxr-xr-x  2 root root  user_u:object_r:user_home_dir_t:s0  4096 May  5  2008 opt
|  drwxr-xr-x  2 root qmail system_u:object_r:home_root_t:s0    4096 Dec 29 13:38 qmail
|  drwxr-xr-x  7 root root  system_u:object_r:var_run_t:s0      4096 Dec 29 18:14 run
|  drwxr-xr-x  5 root root  user_u:object_r:user_home_dir_t:s0  4096 Dec 29 14:17 spool
|  drwxrwxrwt  3 root root  system_u:object_r:tmp_t:s0          4096 Dec 29 18:06 tmp
`----

        Every time "semanage user" is run, we get:
,----[ contexts/files/file_contexts.homedirs ]
|  #
|  #
|  # User-specific file contexts, generated via libsemanage
|  # use semanage command to manage system users to change the file_context
|  #
|  #
|
|  #
|  # Home Context for user user_u
|  #
|
|  /home/[^/]*/.+  user_u:object_r:user_home_t:s0
|  /home/[^/]*/\.ssh(/.*)? user_u:object_r:user_home_ssh_t:s0
|  /home/[^/]*/\.gnupg(/.+)?       user_u:object_r:user_gpg_secret_t:s0
|  /home/[^/]*     -d      user_u:object_r:user_home_dir_t:s0
|  /home/lost\+found/.*    <<none>>
|  /home   -d      system_u:object_r:home_root_t:s0
|  /home/\.journal <<none>>
|  /home/lost\+found       -d      system_u:object_r:lost_found_t:s0
|
|
|  #
|  # Home Context for user user_u
|  #
|
|  /var/[^/]*/.+   user_u:object_r:user_home_t:s0
|  /var/[^/]*/\.ssh(/.*)?  user_u:object_r:user_home_ssh_t:s0
|  /var/[^/]*/\.gnupg(/.+)?        user_u:object_r:user_gpg_secret_t:s0
|  /var/[^/]*      -d      user_u:object_r:user_home_dir_t:s0
|  /var/lost\+found/.*     <<none>>
|  /var    -d      system_u:object_r:home_root_t:s0
|  /var/\.journal  <<none>>
|  /var/lost\+found        -d      system_u:object_r:lost_found_t:s0
|
|
|  #
|  # Home Context for user user_u
|  #
|
|  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
|  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
|  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
|  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
|  /var/qmail/lost\+found/.*       <<none>>
|  /var/qmail      -d      system_u:object_r:home_root_t:s0
|  /var/qmail/\.journal    <<none>>
|  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
|  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
|
|
|  #
|  # Home Context for user root
|  #
|
|  /root/.+        root:object_r:sysadm_home_t:s0
|  /root/\.ssh(/.*)?       root:object_r:sysadm_home_ssh_t:s0
|  /root/\.gnupg(/.+)?     root:object_r:sysadm_gpg_secret_t:s0
|  /root   -d      root:object_r:sysadm_home_dir_t:s0
|  /tmp/gconfd-root        -d      root:object_r:sysadm_tmp_t:s0
`----

        This makes the machine unusable when in enforcing mode.
 Additionally, when there was unconfined se-module loaded there were
 unconfined_u instead of user_u as the second and third "users" in this
 file (that is, qmail and whatever added /var/spool).

        You need to hand edit
 $POLICY/contexts/files/file_contexts.homedirs and
 $POLICY/modules/active/file_contexts.homedirs by removing invalid
 entries (mentioning /var).

,----[ semanage user -l ]
|  root            sysadm     s0         s0-s0:c0.c1023                 staff_r sysadm_r system_r
|  staff_u         staff      s0         s0-s0:c0.c1023                 staff_r sysadm_r
|  sysadm_u        sysadm     s0         s0-s0:c0.c1023                 sysadm_r
|  system_u        user       s0         s0-s0:c0.c1023                 system_r
|  unconfined_u    unconfined s0         s0-s0:c0.c1023                 system_r unconfined_r
|  user_u          user       s0         s0                             user_r
`----

,----[ semanage login -l ]
|  __default__               user_u                    s0
|  root                      root                      s0-s0:c0.c1023
|  system_u                  system_u                  s0-s0:c0.c1023
`----

,----[ semodule -l ]
|  dhcp    1.6.0
|  dmidecode       1.3.0
|  gpg     1.6.0
|  mysql   1.8.0
|  netutils        1.6.0
|  ssh     1.10.1
|  sudo    1.3.0
|  tcpd    1.3.0
|  tzdata  1.2.0
`----

        manoj
--
Manoj Srivastava <manoj.srivastava@stdc.com> <srivasta@acm.org>
1024D/BF24424C print 4966 F272 D093 B493 410B  924B 21BA DABB BF24 424C

--
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: [DSE-Dev] [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[Daniel J Walsh]

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

Manoj Srivastava wrote:
> Hi,
> 
>  [Trimming the patch and early discussion]
> 
> On Wed, Jan 07 2009, Daniel J Walsh wrote:
>> Manoj Srivastava wrote:
>>> On Wed, Jan 07 2009, Stephen Smalley wrote:
>>>> As Dan pointed out, the UID_MAX value in login.defs is only used by
>>>> useradd, and is not even strictly enforced (useradd -u 60002 john works
>>>> just fine).  In an environment with a large number of users and a global
>>>> user database, you can certainly exceed 60000 users or you may even
>>>> happen to generate your uids in a manner that happens to put them all in
>>>> the upper part of the uid space.  There are real systems with uids >
>>>> 60000 for real users, yet the login.defs UID_MAX value has not been
>>>> changed on such systems because it is irrelevant and it isn't enforced
>>>> by anything.  So this patch would change behavior of libsemanage on such
>>>> systems in an undesirable manner.  And it wouldn't help with cases like
>>>> oracle where the pseudo user is added via useradd without any specified
>>>> uid at all.
> 
>>>> I think Dan's earlier posting gets to more of the fundamental problems
>>>> with genhomedircon's heuristics for finding home directory locations,
>>>> and we need to address his points if we want it to work in general.
> 
>>>         Fair enough. In that case, I would like to point out that the
>>>  current situation of only checking UID_MIN is causing actual problems
>>>  right now on real user systems, and making genhomedircon to incorrectly
>>>  guess where home directories exist.
> 
>>>         I'll treat this as an imperfect workaround until we fix
>>>  semodule, and then I'll just revert the patch for Debian systems.
> 
>> What does the passwd entry that it is getting fooled by look like?  Does
>> the account really need a real shell?  IE Do people actually login to
>> the home directory?
> 
>         I do not have passwd data from the machine in question, though I
>  can ask. What I do have are the results of fixfiles relabel / :
> 
> ,----[ file contexts in /var ]
> |  drwxr-xr-x 15 root root  system_u:object_r:home_root_t:s0    4096 Dec 29 13:35 .
> |  drwxr-xr-x 21 root root  system_u:object_r:root_t:s0         4096 Dec 29 14:21 ..
> |  drwxr-xr-x  2 root root  user_u:object_r:user_home_dir_t:s0  4096 May  7  2008 backups
> |  drwxr-xr-x  7 root root  user_u:object_r:user_home_dir_t:s0  4096 Dec 29 14:17 cache
> |  drwxr-xr-x 25 root root  user_u:object_r:user_home_dir_t:s0  4096 Dec 29 14:17 lib
> |  drwxrwsr-x  2 root staff user_u:object_r:user_home_dir_t:s0  4096 Mar 11  2008 local
> |  drwxrwxrwt  2 root root  user_u:object_r:user_home_dir_t:s0  4096 Dec 29 18:14 lock
> |  drwxr-xr-x  6 root root  system_u:object_r:var_log_t:s0      4096 Dec 29 18:19 log
> |  drwx------  2 root root  system_u:object_r:lost_found_t:s0  16384 May  5  2008 lost+found
> |  drwxrwsr-x  2 root mail  user_u:object_r:user_home_dir_t:s0  4096 May  5  2008 mail
> |  drwxr-xr-x  2 root root  user_u:object_r:user_home_dir_t:s0  4096 May  5  2008 opt
> |  drwxr-xr-x  2 root qmail system_u:object_r:home_root_t:s0    4096 Dec 29 13:38 qmail
> |  drwxr-xr-x  7 root root  system_u:object_r:var_run_t:s0      4096 Dec 29 18:14 run
> |  drwxr-xr-x  5 root root  user_u:object_r:user_home_dir_t:s0  4096 Dec 29 14:17 spool
> |  drwxrwxrwt  3 root root  system_u:object_r:tmp_t:s0          4096 Dec 29 18:06 tmp
> `----
> 
>         Every time "semanage user" is run, we get:
> ,----[ contexts/files/file_contexts.homedirs ]
> |  #
> |  #
> |  # User-specific file contexts, generated via libsemanage
> |  # use semanage command to manage system users to change the file_context
> |  #
> |  #
> |
> |  #
> |  # Home Context for user user_u
> |  #
> |
> |  /home/[^/]*/.+  user_u:object_r:user_home_t:s0
> |  /home/[^/]*/\.ssh(/.*)? user_u:object_r:user_home_ssh_t:s0
> |  /home/[^/]*/\.gnupg(/.+)?       user_u:object_r:user_gpg_secret_t:s0
> |  /home/[^/]*     -d      user_u:object_r:user_home_dir_t:s0
> |  /home/lost\+found/.*    <<none>>
> |  /home   -d      system_u:object_r:home_root_t:s0
> |  /home/\.journal <<none>>
> |  /home/lost\+found       -d      system_u:object_r:lost_found_t:s0
> |
> |
> |  #
> |  # Home Context for user user_u
> |  #
> |
> |  /var/[^/]*/.+   user_u:object_r:user_home_t:s0
> |  /var/[^/]*/\.ssh(/.*)?  user_u:object_r:user_home_ssh_t:s0
> |  /var/[^/]*/\.gnupg(/.+)?        user_u:object_r:user_gpg_secret_t:s0
> |  /var/[^/]*      -d      user_u:object_r:user_home_dir_t:s0
> |  /var/lost\+found/.*     <<none>>
> |  /var    -d      system_u:object_r:home_root_t:s0
> |  /var/\.journal  <<none>>
> |  /var/lost\+found        -d      system_u:object_r:lost_found_t:s0
> |
> |
> |  #
> |  # Home Context for user user_u
> |  #
> |
> |  /var/qmail/[^/]*/.+     user_u:object_r:user_home_t:s0
> |  /var/qmail/[^/]*/\.ssh(/.*)?    user_u:object_r:user_home_ssh_t:s0
> |  /var/qmail/[^/]*/\.gnupg(/.+)?  user_u:object_r:user_gpg_secret_t:s0
> |  /var/qmail/[^/]*        -d      user_u:object_r:user_home_dir_t:s0
> |  /var/qmail/lost\+found/.*       <<none>>
> |  /var/qmail      -d      system_u:object_r:home_root_t:s0
> |  /var/qmail/\.journal    <<none>>
> |  /var/qmail/lost\+found  -d      system_u:object_r:lost_found_t:s0
> |  /tmp/gconfd-.*  -d      user_u:object_r:user_tmp_t:s0
> |
> |
> |  #
> |  # Home Context for user root
> |  #
> |
> |  /root/.+        root:object_r:sysadm_home_t:s0
> |  /root/\.ssh(/.*)?       root:object_r:sysadm_home_ssh_t:s0
> |  /root/\.gnupg(/.+)?     root:object_r:sysadm_gpg_secret_t:s0
> |  /root   -d      root:object_r:sysadm_home_dir_t:s0
> |  /tmp/gconfd-root        -d      root:object_r:sysadm_tmp_t:s0
> `----
> 
>         This makes the machine unusable when in enforcing mode.
>  Additionally, when there was unconfined se-module loaded there were
>  unconfined_u instead of user_u as the second and third "users" in this
>  file (that is, qmail and whatever added /var/spool).
> 
>         You need to hand edit
>  $POLICY/contexts/files/file_contexts.homedirs and
>  $POLICY/modules/active/file_contexts.homedirs by removing invalid
>  entries (mentioning /var).
> 
> ,----[ semanage user -l ]
> |  root            sysadm     s0         s0-s0:c0.c1023                 staff_r sysadm_r system_r
> |  staff_u         staff      s0         s0-s0:c0.c1023                 staff_r sysadm_r
> |  sysadm_u        sysadm     s0         s0-s0:c0.c1023                 sysadm_r
> |  system_u        user       s0         s0-s0:c0.c1023                 system_r
> |  unconfined_u    unconfined s0         s0-s0:c0.c1023                 system_r unconfined_r
> |  user_u          user       s0         s0                             user_r
> `----
> 
> ,----[ semanage login -l ]
> |  __default__               user_u                    s0
> |  root                      root                      s0-s0:c0.c1023
> |  system_u                  system_u                  s0-s0:c0.c1023
> `----
> 
> ,----[ semodule -l ]
> |  dhcp    1.6.0
> |  dmidecode       1.3.0
> |  gpg     1.6.0
> |  mysql   1.8.0
> |  netutils        1.6.0
> |  ssh     1.10.1
> |  sudo    1.3.0
> |  tcpd    1.3.0
> |  tzdata  1.2.0
> `----
> 
>         manoj
> --
> Manoj Srivastava <manoj.srivastava@stdc.com> <srivasta@acm.org>
> 1024D/BF24424C print 4966 F272 D093 B493 410B  924B 21BA DABB BF24 424C
> 
> --
> 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.
Well semanage is supposed to prevent this from happening.  I think the
latest version does this.  I looks for a conflicting regex for the home
root directory and then refuses to add the home dir.

But the one you seem to be using is broken.  I think fixing the
passwd/homedir entry to use a shell of /sbin/nologin or /bin/false will
fix your problem
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAklmH08ACgkQrlYvE4MpobPGxQCgi5FfF6HalhDacq9nCh5PHANU
zUwAn2eGztoFQAJ47Sxs8KTMKc5M4bWb
=nEn7
-----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: [DSE-Dev] [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs

[selinux]

Sorry for breaking into your conversation,
but it was me who posted the bugreport.

(My name is Alexey G. Shpagin, I am system administrator and I am posting
here for a first time.)

On Thu, Jan 08, 2009 at 08:33:54AM -0600, Manoj Srivastava wrote:
> Hi,
> 
>  [Trimming the patch and early discussion]
> 
> On Wed, Jan 07 2009, Daniel J Walsh wrote:
> > Manoj Srivastava wrote:
> >> On Wed, Jan 07 2009, Stephen Smalley wrote:
> >>> As Dan pointed out, the UID_MAX value in login.defs is only used by
> >>> useradd, and is not even strictly enforced (useradd -u 60002 john works
> >>> just fine).  In an environment with a large number of users and a global
> >>> user database, you can certainly exceed 60000 users or you may even
> >>> happen to generate your uids in a manner that happens to put them all in
> >>> the upper part of the uid space.  There are real systems with uids >
> >>> 60000 for real users, yet the login.defs UID_MAX value has not been
> >>> changed on such systems because it is irrelevant and it isn't enforced
> >>> by anything.  So this patch would change behavior of libsemanage on such
> >>> systems in an undesirable manner.  And it wouldn't help with cases like
> >>> oracle where the pseudo user is added via useradd without any specified
> >>> uid at all.

I should note, that useradd -u 52 john works not even worse. What about the
systems, that have legal user uids below UID_MIN?
UID_MIN is also not enforced anywhere except the automatic uid generation in
useradd or maybe some custom pam modules (don't know of any).

> 
> >>> I think Dan's earlier posting gets to more of the fundamental problems
> >>> with genhomedircon's heuristics for finding home directory locations,
> >>> and we need to address his points if we want it to work in general.
> 
> >>         Fair enough. In that case, I would like to point out that the
> >>  current situation of only checking UID_MIN is causing actual problems
> >>  right now on real user systems, and making genhomedircon to incorrectly
> >>  guess where home directories exist.
> 
> >>         I'll treat this as an imperfect workaround until we fix
> >>  semodule, and then I'll just revert the patch for Debian systems.
> 
> > What does the passwd entry that it is getting fooled by look like?  Does
> > the account really need a real shell?  IE Do people actually login to
> > the home directory?
> 
>         I do not have passwd data from the machine in question, though I
>  can ask. What I do have are the results of fixfiles relabel / :

The accounts were created not by hand, but by (pre|post)install script of qmail
package. That script is not perfect (and already have some minor issues), but
still I'm not sure if adding system users with /bin/sh was done by mistake
or for some reason, yet to be checked.
In the case you are still curious, here they are:

alias:x:64010:65534:qmail alias,,,:/var/qmail/alias:/bin/sh
qmaild:x:64011:65534:qmail daemon,,,:/var/qmail:/bin/sh
qmails:x:64012:64010:qmail send,,,:/var/qmail:/bin/sh
qmailr:x:64013:64010:qmail remote,,,:/var/qmail:/bin/sh
qmailq:x:64014:64010:qmail queue,,,:/var/qmail:/bin/sh
qmaill:x:64015:65534:qmail log,,,:/var/qmail:/bin/sh
qmailp:x:64016:65534:qmail pw,,,:/var/qmail:/bin/sh

What I can't keep in mind anymore is the way I want it to be:

 1. Every nontrivial behavior (you call it heuristics) of libsemanage must be
    documented prior deployment (and documentation be referenced from at least
    semanage(8) man page). It looks like libsemanage already have enough
    heuristics and deserves it's own manpage in (8).
      I think, that the way of dependency of something perceived as component
    of selinux (that stated to have no relation to UNIX uids and accounts) on 
    records from /etc/passwd is pretty nontrivial. Something tells me, that it
    should somehow map UNIX accounts to selinux labels, but I can only guess about
    what is the exact process.
      Even more, hidden dependency on some_but_not_others config parameters from
    logins.defs is just the example of nontrivial and counterintuitional behavior.
    (It will be easier for me (without need for sources) to track down the issue if
    UID_MIN were not used too, or UID_(MIN|MAX) were used both).

 2. It is hard for me to determine, what are the login.defs' UID_MIN and UID_MAX for,
    and especially what they are not.
    The (only|best) way they can be used is during the initial configuration of
    libsemanage by postinstallation scripts that create SEPARATE configuration
    file for libsemanage and copy there all the needed settings from whatever
    you want. Maybe asking the administrator some questions. Also allowing him
    to reconfigure that again at his request.
    
    Only in separate configs should exists that UID_MIN, optional UID_MAX (and maybe
    also useful UIDS_EXCLUDE, USERS_EXCLUDE, and even better - GIDS_EXCLUDE,
    GROUPS_EXCLUDE) that will affect applications deploying libsemanage.
    They should be documented also, but if not, it will already be easier to guess
    their effect if they will live inside something like
    /etc/selinux/libsemanage/automatic_accounts_mapping.conf
    (well, at least easier for me).
    And it will be natural for [new] system administrator, who changed /etc/login.defs
    in some way, not to hope that it will affect selinux components in some other way.
    It will be natural for him to make an attempt to find corresponding selinux
    configuration parameters to correct them accordingly
    (if that seem appropriate for him).

Still I wonder whether some other selinux components (in other libraries) depend on
some old (more traditional than SELinux) UNIX configuration files in some way...

Consider this as a subjective opinion of a mediocre system administrator, that have
to deal with your creatures.
Thanks for your time and many thanks for that creatures.

--
Alexey G. Shpagin
System administrator
Volga Telecom

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