[DSE-Dev] [libsemanage] Also check for the uppoer bound on user ids in /etc/login.defs
Manoj Srivastava
manoj.srivastava at stdc.com
Tue Jan 6 14:51:32 UTC 2009
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 at 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 at 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 at stdc.com> <srivasta at acm.org>
1024D/BF24424C print 4966 F272 D093 B493 410B 924B 21BA DABB BF24 424C
More information about the SELinux-devel
mailing list