* Stefan Hajnoczi (stefa...@redhat.com) wrote:
> virtiofsd runs as root but only needs a subset of root's Linux
> capabilities(7). As a file server its purpose is to create and access
> files on behalf of a client. It needs to be able to access files with
> arbitrary uid/gid owners. It also needs to be create device nodes.
>
> Introduce a Linux capabilities(7) whitelist and drop all capabilities
> that we don't need, making the virtiofsd process less powerful than a
> regular uid root process.
>
> # cat /proc/PID/status
> ...
> Before After
> CapInh: 0000000000000000 0000000000000000
> CapPrm: 0000003fffffffff 00000000880000df
> CapEff: 0000003fffffffff 00000000880000df
> CapBnd: 0000003fffffffff 0000000000000000
> CapAmb: 0000000000000000 0000000000000000
>
> Note that file capabilities cannot be used to achieve the same effect on
> the virtiofsd executable because mount is used during sandbox setup.
> Therefore we drop capabilities programmatically at the right point
> during startup.
>
> This patch only affects the sandboxed child process. The parent process
> that sits in waitpid(2) still has full root capabilities and will be
> addressed in the next patch.
>
> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
Looks reasonable to me; I can't see any capabilities in the manpage that
you're missing that make sense.
They also look old enough not to be a problem with reasonably old
systems.
Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> ---
> tools/virtiofsd/passthrough_ll.c | 38 ++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/tools/virtiofsd/passthrough_ll.c
> b/tools/virtiofsd/passthrough_ll.c
> index 4c35c95b25..af97ba1c41 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -2695,6 +2695,43 @@ static void setup_mounts(const char *source)
> close(oldroot);
> }
>
> +/*
> + * Only keep whitelisted capabilities that are needed for file system
> operation
> + */
> +static void setup_capabilities(void)
> +{
> + pthread_mutex_lock(&cap.mutex);
> + capng_restore_state(&cap.saved);
> +
> + /*
> + * Whitelist file system-related capabilities that are needed for a file
> + * server to act like root. Drop everything else like networking and
> + * sysadmin capabilities.
> + *
> + * Exclusions:
> + * 1. CAP_LINUX_IMMUTABLE is not included because it's only used via
> ioctl
> + * and we don't support that.
> + * 2. CAP_MAC_OVERRIDE is not included because it only seems to be
> + * used by the Smack LSM. Omit it until there is demand for it.
> + */
> + capng_setpid(syscall(SYS_gettid));
> + capng_clear(CAPNG_SELECT_BOTH);
> + capng_updatev(CAPNG_ADD, CAPNG_PERMITTED | CAPNG_EFFECTIVE,
> + CAP_CHOWN,
> + CAP_DAC_OVERRIDE,
> + CAP_DAC_READ_SEARCH,
> + CAP_FOWNER,
> + CAP_FSETID,
> + CAP_SETGID,
> + CAP_SETUID,
> + CAP_MKNOD,
> + CAP_SETFCAP);
> + capng_apply(CAPNG_SELECT_BOTH);
> +
> + cap.saved = capng_save_state();
> + pthread_mutex_unlock(&cap.mutex);
> +}
> +
> /*
> * Lock down this process to prevent access to other processes or files
> outside
> * source directory. This reduces the impact of arbitrary code execution
> bugs.
> @@ -2705,6 +2742,7 @@ static void setup_sandbox(struct lo_data *lo, struct
> fuse_session *se,
> setup_namespaces(lo, se);
> setup_mounts(lo->source);
> setup_seccomp(enable_syslog);
> + setup_capabilities();
> }
>
> /* Raise the maximum number of open file descriptors */
> --
> 2.25.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
