[CRIU] [PATCH] security: set suid flag on crtools and check real uid on dump/restore

[Ruslan Kuprieiev]

On 02.10.2013 18:27, Ruslan Kuprieiev wrote:
> On 02.10.2013 14:03, Pavel Emelyanov wrote:
>> On 10/02/2013 05:54 PM, Ruslan Kuprieiev wrote:
>>> On 02.10.2013 13:25, Pavel Emelyanov wrote:
>>>> On 10/02/2013 05:00 PM, Ruslan Kuprieiev wrote:
>>>>> Hi!
>>>>>
>>>>> Lets set suid flag on crtools, so non-root users could dump/restore
>>>>> their own tasks and start service for their own tasks. On start criu
>>>>> will get it's real uid and will allow user to dump/restore only tasks
>>>>> that he own.
>>>>>
>>>>> Signed-off-by: Ruslan Kuprieiev <kupruser at gmail.com>
>>>>>
>>>> I don't quite understand the logic behind security_init() + 
>>>> restrict_uid()
>>>> and the need in two uids stores in security.c
>>> I think we can extend security_init later with some extra features (but
>>> I don't know with which, though:)). Also checkpatch.pl was mad about
>>> initializing static variables with zeros:).
>>>
>>> And restrict_uid() will be used very often in cr
>>> I think we may need to remember real uid, so  if non-root will start
>>> service, he won't be able to change his "effective" uid and 
>>> dump/restore
>>> tasks with other uids.
>> He will not be able to do it anyway. The restrict_uid() is not "let me
>> dump this uid", but "don't even try to dump anything but this".
>>
>>> It looks better to me, than resolving this
>>> situation in cr-service. Also i do think that ruid may be in handy 
>>> later.
>>> Or just use getuid() every time, instead of declaring second uid?
>> I thought that we just restrict_uid(getuid()) on crtools start and in
>> service child req-setup and that's it.
> If non-root starts service, on crtools we call restrict_uid(getuid()) 
> and now he is allowed to dump only his own tasks. Than, he gets a 
> request(through rpc) to dump task with other uid and we call 
> restcrict_uid(uid). So we need to check getuid() at restrict_uid(), so 
> root would be able to restrict to any uid, but non-root won't be able 
> to do this. Something like this:
>
> void restrict_uid(uid)
> {
>     if (getuid() == 0)
>         dumper_uid = uid;
> }
>
>
beter like this:

void restrict_uid(uid)
{
     if (getuid() == 0 || getuid() == uid)
         dumper_uid = uid;
}

so we will be able to use it in crtools in the very beginning.