[CRIU] [PATCH] security: check_ids - return true if [se]?[ug]id is the same as task id

[Pavel Emelyanov]

On 06/16/2014 04:10 PM, Ruslan Kuprieiev wrote:
> On 16.06.2014 14:23, Pavel Emelyanov wrote:
>> On 06/16/2014 09:55 AM, Ruslan Kuprieiev wrote:
>>> And again sorry for necromailing=).
>>>
>>> After a bit more thinking, here is what I figured out.
>>>
>>> Current check if(crid == id && crid == eid && crid == sid) doesn't work
>>> right.
>>> Andrew was able to show it in "[CRIU] RPC support for --shell-job
>>> missing on restore"
>>> thread.
>>> I wasn't able to make it work only with crid available.
>>>
>>> Andrew also suggested that we should be able to dump/restore task if we
>>> are able to kill it. He also attached a function from the kernel:
>>>
>>> static int kill_ok_by_cred(struct task_struct *t)
>>> {
>>>           const struct cred *cred = current_cred();
>>>           const struct cred *tcred = __task_cred(t);
>>>
>>>           if (uid_eq(cred->euid, tcred->suid) ||
>>>               uid_eq(cred->euid, tcred->uid)  ||
>>>               uid_eq(cred->uid,  tcred->suid) ||
>>>               uid_eq(cred->uid,  tcred->uid))
>>>                   return 1;
>>>
>>>           if (ns_capable(tcred->user_ns, CAP_KILL))
>>>                   return 1;
>>>
>>>           return 0;
>>> }
>> Not sure this would work for us. If I'm user 12 dump a task, then
>> edit its creds to have id == 12, suid == 0 and caps == <all>, then
>> I'm effectively allowed to restore a process with root caps.
> 
> Oh, I see. You are talking about manipulating images?

Exactly.

> Yes, that looks like a potential big security issue.
> 
>> This is bad.
>>
>> AFAIU the problem we're trying to fix is called "we cannot dump and
>> restore the suid-ed process", isn't it?
> 
> Yes, kind of. Process is sgid-ed, but belongs to user's gid == process 
> gid. And that user
> can't dump his own process. It doesn't look right.
> 
> Maybe it's time to think about securing images? Maybe, encrypt it with smth?

I have two possibilities in my head. The first is to check, that
images belong to a directory the user that requests restore doesn't
have write access to. In that case we probably can imply, that the
images were not modified.

The second is more sophisticated -- when restoring a process whose
suid differs from the uid one, we should check that a) the executable
and libraries we link to it belong to this suid and b) the code that
would get executed sits in one of these files. But this way looks
quite nasty.

Thanks,
Pavel