Re: [Xen-devel] [PATCH] docs/qemu-deprivilege: Revise and update with status and future plans

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 03/27/2018 02:33 PM, Ian Jackson wrote:
>>>> +### Namespaces for unused functionality
> ...
>>>> +    unshare(CLONE_NEWNS | CLONE_NEWIPC)
>>>
>>> This would mean we would have to pass qemu fds for both the network
>>> tap devices and any vnc consoles.  That makes life considerably more
>>> complicated.  I think we should perhaps revisit this upstream.
>>
>> You haven't read this very carefully (nor do you seem to have read the
>> discussion with Ross before responding).  I split the "Namespaces"
>> restriction Ross suggested internally to us into two parts:
>>  - Namespace restrictions for unused functionality
>>  - Network namespacing
> 
> Oh, sorry.  Yes.

Actually, sorry for being a bit 'pointy' here; upon reflection it
occured to me that you probably actually did read the whole thread, but
when you went back to respond you meant to respond to the other section,
but stopped scanning when you got a match with "namespace".

>> I now realize that it might be that the limit will follow the current
>> uid of the process, in which case "setrlimit -> setuid" might have the
>> expected behavior.  But a quick Google search shows that the interaction
>> of RLIMIT_NPROC and setuid() is tricky[1][2], and may vary from
>> implementation to implementation; relying on the interaction to be
>> correct (and stay correct) seems somewhat risky (unless POSIX has
>> explicitly documented what should happen in that case, which again a
>> quick Google search hasn't turned up).
> 
> RLIMIT_NPROC is not in SuS:
>  http://pubs.opengroup.org/onlinepubs/9699919799/functions/setrlimit.html
> 
> FreeBSD does not have any of this madness:
>  https://www.unix.com/man-page/freebsd/2/setuid/
> 
> Anyway IMO we should set RLIMIT_NPROC after fork and before execve.
> If this causes setuid to fail in qemu, qemu will crash.  But this
> could only be the case if the new uid already has other processes,
> which it shouldn't do.  (If it does, the new uid is probably
> contaminated by a previous domain with the same domid.)

I was more worried about the limit not having the expected effect after
the setuid().

>>>> +### libxl UID cleanup
>>> ...
>>>> +kill(-1,sig) sends a signal to "every process to which the calling
>>>> +process has permission to send a signal".  So in theory:
>>>> +  setuid(X)
>>>> +  kill(-1,KILL)
>>>> +should do the trick.
>>>
>>> We need to check whether a malicious qemu process could kill this
>>> one.
>>
>> Hmm, in theory it probably could.
> 
> I could find nothing in SuS explaining when process A may send signals
> to process B.  So I resorted to the BSD manpages:
> 
>   https://www.unix.com/man-page/freebsd/2/kill/
> 
>   For a process to have permission to send a signal to a process
>   designated by pid, the user must be the super-user, or the real or
>   saved user ID of the receiving process must match the real or
>   effective user ID of the sending process.

The text of both the FreeBSD and Linux man pages looks to be copied
verbatim from [1].

[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/kill.html

> Also kill(-1,) does not signal the sender.

Linux is the same.

> My analysis:
> 
>                                           euid   ruid   suid
> 
>   compared for receiving process                 ====   ====
>   compared for sending process            ====   ====
> 
>   possible rogue processes                qid    qid    qid
> 
>   unrelated rootish processes             0      any    any
>      or                                   any    0      any
>      or                                   any    any    0
> 
>   killer (our pet issuing kill)
>      so rogues can't kill killer                 !=qid  !=qid
>      so it doesn't kill randomly          !=0    !=0
>      therefore:                           qid    pet    0
> 
> We will have to reserve one uid `pet' specially for this nonsense.  We
> call setresuid(2) to switch uids, and then kill(-1,9) and _exit(0).
> The kill() call will kill other processes with uid `pet'.  So we take
> a global lock while we do this.
> 
> `pet' could be a uid associated with a reserved domid, eg dom0.

Right -- it looks like that could work.  I hadn't initially noticed the
{RUID, SUID} => {RUID, EUID} distinction.

It's kind of hard to believe this is so difficult to pull off.

Should we post this to that forum, for the benefit of other people who
end up finding the same discussion? :-)

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel