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

[Pavel Emelyanov]

On 07/04/2014 06:39 PM, Ruslan Kuprieiev wrote:
> 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?

Yes.

> 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.

My idea was that if the task we dump belongs to the same used that dumps
it, groups can be any. But on restore, we should check that user didn't
add "wrong" groups into the image.

Thanks,
Pavel