-----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@xxxxxxxxxx>
>>>
>>> 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@xxxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxxxxx with
the words "unsubscribe selinux" without quotes as the message.
