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

[Andrew Vagin]

On Mon, Jun 16, 2014 at 07:18:10PM +0400, Pavel Emelyanov wrote:
> 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.

We was talking with you about the third one. Images are created from a
suid user. On restore criu checks that images are belonged to this user.

Only root can change file owners, so it looks secure.

> 
> Thanks,
> Pavel
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu