[CRIU] [PATCH] security: check additional groups

[Ruslan Kuprieiev]

On 02.07.2014 15:17, Ruslan Kuprieiev wrote:
> On 02.07.2014 14:58, Pavel Emelyanov wrote:
>>> -void restrict_uid(unsigned int uid, unsigned int gid)
>>> +int restrict_uid(unsigned int uid, unsigned int gid)
>>>   {
>>> -    pr_info("Restrict C/R with %u:%u uid\n", uid, gid);
>>> +    struct passwd *pwd;
>>> +    unsigned int buf[NGROUPS_MAX];
>>> +    int nbuf;
>>> +
>>> +    pr_info("Restrict C/R with %u:%u uid:gid\n", uid, gid);
>>>       cr_uid = uid;
>>>       cr_gid = gid;
>>> +
>>> +    pwd = getpwuid(uid);
>>> +    if (!pwd) {
>>> +        pr_perror("Can't get password file entry");
>>> +        return -1;
>>> +    }
>>> +
>>> +    nbuf = NGROUPS_MAX;
>>> +    if (getgrouplist(pwd->pw_name, pwd->pw_gid, buf, &nbuf) < 0) {
>> There's no need in getting the caller's groups. It's enough just
>> to check the group it currently lives in.
>
> User's current group may not match all gids of the process. Just as it 
> was with
> that game. When user's group was 1000, but process had (1000,20,20).
>

Oh, are you suggesting to check that process grouplist contains caller's 
gid?

> And we should check that user isn't restoring process which has more 
> groups
> than user already has, shouldn't we?
>
>>> +        pr_perror("Can't get group list");
>>> +        return -1;
>>> +    }
>>> +
>>> +    cr_ngroups = nbuf;
>>> +    cr_groups = xmalloc(cr_ngroups*sizeof(*cr_groups));
>>> +    if (!cr_groups)
>>> +        return -1;
>>> +
>>> +    memcpy(cr_groups, buf, cr_ngroups*sizeof(*cr_groups));
>>> +
>>> +    return 0;
>>>   }
>