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

[Ruslan Kuprieiev]

On 04.07.2014 17:47, Pavel Emelyanov wrote:
> 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.

Yep, I meant it too.

>
> Thanks,
> Pavel
>
>