[CRIU] [PATCH 2/2] security: restrict criu with groups

[Ruslan Kuprieiev]

On 04.07.2014 17:26, Ruslan Kuprieiev wrote:
> On 04.07.2014 17:23, Pavel Emelyanov wrote:
>> On 07/04/2014 06:20 PM, Ruslan Kuprieiev wrote:
>>> On 04.07.2014 17:10, Pavel Emelyanov wrote:
>>>>> @@ -62,13 +134,13 @@ static bool check_caps(u32 *inh, u32 *eff, 
>>>>> u32 *prm)
>>>>>    bool may_dump(struct proc_status_creds *creds)
>>>>>    {
>>>>>        return check_ids(cr_uid, creds->uids[0], creds->uids[1], 
>>>>> creds->uids[2]) &&
>>>>> -        check_ids(cr_gid, creds->gids[0], creds->gids[1], 
>>>>> creds->gids[2]) &&
>>>>> +        check_gids(creds->gids[0], creds->gids[1], 
>>>>> creds->gids[2], creds->groups, creds->ngroups) &&
>>>> Getting groups on dump from proc file is not right. First of all, 
>>>> proc shows
>>>> only first 32 groups. And to address that, we get groups out of 
>>>> parasite code,
>>>> so they are ready some time during the dump.
>>> Oh, I didn't know that.
>>> Will use getgrouplist.
>> getgrouplist on dump is also not good. You should use the groups
>> obtained from the task.
>
> Could you tell me why it is not good, please?
>

Because of namespaces, right? Because /etc/group may depend on namespace?

Btw, it looks wrong to insert parasite before doing this check.

Maybe we don't need process groups on dump? Because, if uid check is passed
and gids of the task are in the grouplist of the caller, then groups 
should match
too.

>>
>>> Thanks.
>>>
>>>>> check_caps(creds->cap_inh, creds->cap_eff, creds->cap_prm);
>>>>>    }
>>>>>       bool may_restore(CredsEntry *creds)
>>>>>    {
>>>>>        return check_ids(cr_uid, creds->uid, creds->euid, 
>>>>> creds->suid) &&
>>>>> -        check_ids(cr_gid, creds->gid, creds->egid, creds->sgid) &&
>>>>> +        check_gids(creds->gid, creds->egid, creds->sgid, 
>>>>> creds->groups, creds->n_groups) &&
>>>>>            check_caps(creds->cap_inh, creds->cap_eff, 
>>>>> creds->cap_prm);
>>>>>    }
>>>>>
>>> .
>>>
>