[CRIU] [PATCH] security: check additional gids

Ruslan Kuprieiev kupruser at gmail.com
Tue Jun 24 06:05:18 PDT 2014


On 24.06.2014 15:13, Pavel Emelyanov wrote:
> On 06/23/2014 11:04 AM, Ruslan Kuprieiev wrote:
>> Currently, we only check if process gids match primary gid of user.
>> But user may have some additional groups. So lets check them too.
> I believe you inverted this in the patch. We should check for additional
> groups from image to be accessible by the caller, shouldn't we?

Oh! You are right!
Sorry, will fix and resend.

>
>> Signed-off-by: Ruslan Kuprieiev <kupruser at gmail.com>
>> ---
>>    cr-service.c      |  3 ++-
>>    crtools.c         |  3 ++-
>>    include/crtools.h |  2 +-
>>    security.c        | 74
>> +++++++++++++++++++++++++++++++++++++++++++++++--------
>>    4 files changed, 69 insertions(+), 13 deletions(-)
>>
>> -void restrict_uid(unsigned int uid, unsigned int gid)
>> +int restrict_uid(unsigned int uid, unsigned int gid)
>>    {
>> +    struct passwd *pwd;
>> +
>>        pr_info("Restrict C/R with %u:%u uid\n", uid, gid);
>>        cr_uid = uid;
>> -    cr_gid = gid;
>> +
>> +    pwd = getpwuid(uid);
>> +    if (!pwd) {
>> +        pr_perror("Can't get user name");
>> +        return -1;
>> +    }
>> +
>> +    cr_gids_num = NGROUPS_MAX;
>> +    if (getgrouplist(pwd->pw_name, gid, cr_gids, &cr_gids_num) < 0) {
> These two reveal non run-time information about the caller, unlike
> the rest of security code that work on the actual info.
>
>> +        pr_perror("Can't get group list of the user");
>> +        return -1;
>> +    }
>> +
>> +    return 0;
>>    }



More information about the CRIU mailing list