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

[Daniel P. Berrange]

On Fri, Jul 28, 2017 at 02:10:37PM +0200, Eduardo Otubo wrote:
> This patch introduces the new argument
> [,elevateprivileges=allow|deny|children] to the `-sandbox on'. It allows
> or denies Qemu process to elevate its privileges by blacklisting all
> set*uid|gid system calls. The 'children' option will let forks and
> execves run unprivileged.
> 
> Signed-off-by: Eduardo Otubo <address@hidden>
> ---
>  include/sysemu/seccomp.h |  1 +
>  qemu-options.hx          |  9 ++++++---
>  qemu-seccomp.c           | 29 +++++++++++++++++++++++++++++
>  vl.c                     | 22 ++++++++++++++++++++++
>  4 files changed, 58 insertions(+), 3 deletions(-)
> 
> diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
> index 7a7bde246b..e6e78d85ce 100644
> --- a/include/sysemu/seccomp.h
> +++ b/include/sysemu/seccomp.h
> @@ -16,6 +16,7 @@
>  #define QEMU_SECCOMP_H
>  
>  #define OBSOLETE    0x0001
> +#define PRIVILEGED  0x0010

Err, this is hex, but you seem to be treating it as a binary
string. It would be better expressed as

  #define OBSOLETE    (1 << 0)
  #define PRIVILEGED  (1 << 1)
  #define ....        (1 << 2)
  #define ....        (1 << 3)
  #define ....        (1 << 4)


>  
> +        value = qemu_opt_get(opts, "elevateprivileges");
> +        if (value) {
> +            if (strcmp(value, "deny") == 0) {
> +                seccomp_opts |= PRIVILEGED;
> +            }
> +            if (strcmp(value, "children") == 0) {
> +                seccomp_opts |= PRIVILEGED;
> +
> +                /* calling prctl directly because we're
> +                 * not sure if host has CAP_SYS_ADMIN set*/
> +                if (prctl(PR_SET_NO_NEW_PRIVS, 1)) {
> +                    error_report("failed to set no_new_privs "
> +                                 "aborting");
> +                }

The prctl() really ought to be done in seccomp_start IMHO.

> +            }

Also it should report an error for invalid 'value' strings.

> +        }
> +
>          if (seccomp_start(seccomp_opts) < 0) {
>              error_report("failed to install seccomp syscall filter "
>                           "in the kernel");
> -- 
> 2.13.3
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|