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

[Eduardo Otubo]

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?

> 
> > 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. 

> 
> 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.

-- 
Eduardo Otubo
ProfitBricks GmbH