Re: [Qemu-devel] [PATCH 3/5] seccomp: add elevateprivileges argument to command line

[Paolo Bonzini]

On 14/03/2017 13:32, Eduardo Otubo wrote:
> On Tue, Mar 14, 2017 at 01=13=15PM +0100, Paolo Bonzini wrote:
>>
>>
>> On 14/03/2017 12:52, Daniel P. Berrange wrote:
>>>>>  DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
>>>>> -    "-sandbox on[,obsolete=allow]  Enable seccomp mode 2 system call 
>>>>> filter (default 'off').\n" \
>>>>> -    "                              obsolete: Allow obsolete system 
>>>>> calls",
>>>>> +    "-sandbox on[,obsolete=allow][,elevateprivileges=deny]\n" \
>>>>> +    "                               Enable seccomp mode 2 system call 
>>>>> filter (default 'off').\n" \
>>>>> +    "                               obsolete: Allow obsolete system 
>>>>> calls\n" \
>>>>> +    "                               elevateprivileges: avoids Qemu 
>>>>> process to elevate its privileges by blacklisting all set*uid|gid system 
>>>>> calls",
>>>> Why allow these by default?
>>> The goal is that '-sandbox on' should not break *any* QEMU feature. It
>>> needs to be a safe thing that people can unconditionally turn on without
>>> thinking about it.
>>
>> Sure, but what advantages would it provide if the default blacklist does
>> not block anything meaningful?  At the very least, spawn=deny should
>> default elevateprivileges to deny too.
>>
>> I think there should be a list (as small as possible) of features that
>> are sacrificed by "-sandbox on".
> 
> Can you give an example of such a list?

Well, anything that makes "-sandbox on" more than security theater...  I
would consider it a necessary evil, not a good thing to have such a list.

Due to NO_NEW_PRIVS, I think non-migration fork/exec should be on the
list, but hopefully nothing else.

>>> The QEMU bridge helper  requires setuid privs, hence
>>> elevated privileges needs to be permitted by default.
>>
>> QEMU itself should not be getting additional privileges, only the helper
>> and in turn the helper or ifup scripts can be limited through MAC.  The
>> issue is that seccomp persists across execve.
>>
>> Currently, unprivileged users are only allowed to install seccomp
>> filters if no_new_privs is set.  Would it make sense if seccomp filters
>> without no_new_privs succeeded, except that the filter would not persist
>> across execve of binaries with setuid, setgid or file capabilities?
> 
> Yes, it does make sense. Using seccomp_attr_set() and the flag
> SCMP_FLTATR_CTL_NNP we can disable NO_NEW_PRIVS. 

That however in turn requires CAP_SYS_ADMIN.  Without kernel changes,
it's not possible.

Paolo

>>
>> Then the spawn option could be a tri-state with the choice of allow,
>> deny and no_new_privs:
>>
>> - elevateprivileges=allow,spawn=allow: legacy for old kernels
>> - elevateprivileges=deny,spawn=allow: can run privileged helpers
>> - elevateprivileges=deny,spawn=deny: cannot run helpers at all
>> - elevateprivileges=deny,spawn=no_new_privs: can run unprivileged
>> helpers only
> 
> I think it does look interesting to me this model.
>