[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] virtfs-proxy-helper: switch from libcap to libcap-ng
|
From: |
Greg Kurz |
|
Subject: |
Re: [PATCH] virtfs-proxy-helper: switch from libcap to libcap-ng |
|
Date: |
Fri, 29 Nov 2019 13:32:41 +0100 |
On Fri, 29 Nov 2019 12:16:32 +0100
Paolo Bonzini <address@hidden> wrote:
> virtfs-proxy-helper is the only user of libcap; everyone else is using
> the simpler libcap-ng API. Switch and remove the configure code to
> detect libcap.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
Nice. :)
Reviewed-by: Greg Kurz <address@hidden>
Paolo,
I can take this through my 9p tree if you want. Otherwise,
Acked-by: Greg Kurz <address@hidden>
Also, this calls for some extra cleanup in travis.yml and gitlab-ci.yml
which were recently amended by Thomas to install libcap-dev.
commit c269447f78b7cfb0e85d14bc7bb8cb0d25d19781
Author: Thomas Huth <address@hidden>
Date: Thu Sep 5 13:33:46 2019 +0200
travis.yml: Install libcap-dev for testing virito-9p
and
commit e7dc804ef0d7cac9ac8b4a1324ab7dbfafb55704
Author: Thomas Huth <address@hidden>
Date: Thu Sep 5 12:36:50 2019 +0200
gitlab-ci.yml: Install libattr-devel and libcap-devel to test virtio-9p
> configure | 18 +------
> fsdev/virtfs-proxy-helper.c | 100 ++++++++++++++++--------------------
> 2 files changed, 46 insertions(+), 72 deletions(-)
>
> diff --git a/configure b/configure
> index afe9393f04..2216662bf6 100755
> --- a/configure
> +++ b/configure
> @@ -3863,22 +3863,6 @@ else
> mpathpersist=no
> fi
>
> -##########################################
> -# libcap probe
> -
> -if test "$cap" != "no" ; then
> - cat > $TMPC <<EOF
> -#include <stdio.h>
> -#include <sys/capability.h>
> -int main(void) { cap_t caps; caps = cap_init(); return caps != NULL; }
> -EOF
> - if compile_prog "" "-lcap" ; then
> - cap=yes
> - else
> - cap=no
> - fi
> -fi
> -
> ##########################################
> # pthread probe
> PTHREADLIBS_LIST="-pthread -lpthread -lpthreadGC2"
> @@ -6204,7 +6188,7 @@ if test "$want_tools" = "yes" ; then
> fi
> if test "$softmmu" = yes ; then
> if test "$linux" = yes; then
> - if test "$virtfs" != no && test "$cap" = yes && test "$attr" = yes ; then
> + if test "$virtfs" != no && test "$cap_ng" = yes && test "$attr" = yes ;
> then
> virtfs=yes
> tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)"
> else
> diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
> index 6f132c5ff1..0d4de49dcf 100644
> --- a/fsdev/virtfs-proxy-helper.c
> +++ b/fsdev/virtfs-proxy-helper.c
> @@ -13,7 +13,6 @@
> #include <sys/resource.h>
> #include <getopt.h>
> #include <syslog.h>
> -#include <sys/capability.h>
> #include <sys/fsuid.h>
> #include <sys/vfs.h>
> #include <sys/ioctl.h>
> @@ -21,6 +20,7 @@
> #ifdef CONFIG_LINUX_MAGIC_H
> #include <linux/magic.h>
> #endif
> +#include <cap-ng.h>
> #include "qemu-common.h"
> #include "qemu/sockets.h"
> #include "qemu/xattr.h"
> @@ -79,49 +79,10 @@ static void do_perror(const char *string)
> }
> }
>
> -static int do_cap_set(cap_value_t *cap_value, int size, int reset)
> -{
> - cap_t caps;
> - if (reset) {
> - /*
> - * Start with an empty set and set permitted and effective
> - */
> - caps = cap_init();
> - if (caps == NULL) {
> - do_perror("cap_init");
> - return -1;
> - }
> - if (cap_set_flag(caps, CAP_PERMITTED, size, cap_value, CAP_SET) < 0)
> {
> - do_perror("cap_set_flag");
> - goto error;
> - }
> - } else {
> - caps = cap_get_proc();
> - if (!caps) {
> - do_perror("cap_get_proc");
> - return -1;
> - }
> - }
> - if (cap_set_flag(caps, CAP_EFFECTIVE, size, cap_value, CAP_SET) < 0) {
> - do_perror("cap_set_flag");
> - goto error;
> - }
> - if (cap_set_proc(caps) < 0) {
> - do_perror("cap_set_proc");
> - goto error;
> - }
> - cap_free(caps);
> - return 0;
> -
> -error:
> - cap_free(caps);
> - return -1;
> -}
> -
> static int init_capabilities(void)
> {
> /* helper needs following capabilities only */
> - cap_value_t cap_list[] = {
> + int cap_list[] = {
> CAP_CHOWN,
> CAP_DAC_OVERRIDE,
> CAP_FOWNER,
> @@ -130,7 +91,34 @@ static int init_capabilities(void)
> CAP_MKNOD,
> CAP_SETUID,
> };
> - return do_cap_set(cap_list, ARRAY_SIZE(cap_list), 1);
> + int i;
> +
> + capng_clear(CAPNG_SELECT_BOTH);
> + for (i = 0; i < ARRAY_SIZE(cap_list); i++) {
> + if (capng_update(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED,
> + cap_list[i]) < 0) {
> + do_perror("capng_update");
> + return -1;
> + }
> + }
> + if (capng_apply(CAPNG_SELECT_BOTH) < 0) {
> + do_perror("capng_apply");
> + return -1;
> + }
> +
> + /* Prepare effective set for setugid. */
> + for (i = 0; i < ARRAY_SIZE(cap_list); i++) {
> + if (cap_list[i] == CAP_DAC_OVERRIDE) {
> + continue;
> + }
> +
> + if (capng_update(CAPNG_DROP, CAPNG_EFFECTIVE,
> + cap_list[i]) < 0) {
> + do_perror("capng_update");
> + return -1;
> + }
> + }
> + return 0;
> }
>
> static int socket_read(int sockfd, void *buff, ssize_t size)
> @@ -295,14 +283,6 @@ static int setugid(int uid, int gid, int *suid, int
> *sgid)
> {
> int retval;
>
> - /*
> - * We still need DAC_OVERRIDE because we don't change
> - * supplementary group ids, and hence may be subjected DAC rules
> - */
> - cap_value_t cap_list[] = {
> - CAP_DAC_OVERRIDE,
> - };
> -
> *suid = geteuid();
> *sgid = getegid();
>
> @@ -316,11 +296,21 @@ static int setugid(int uid, int gid, int *suid, int
> *sgid)
> goto err_sgid;
> }
>
> - if (uid != 0 || gid != 0) {
> - if (do_cap_set(cap_list, ARRAY_SIZE(cap_list), 0) < 0) {
> - retval = -errno;
> - goto err_suid;
> - }
> + if (uid == 0 && gid == 0) {
> + /* Linux has already copied the permitted set to the effective set.
> */
> + return 0;
> + }
> +
> + /*
> + * All capabilities have been cleared from the effective set. However
> + * we still need DAC_OVERRIDE because we don't change supplementary
> + * group ids, and hence may be subject to DAC rules. init_capabilities
> + * left the set of capabilities that we want in libcap-ng's state.
> + */
> + if (capng_apply(CAPNG_SELECT_CAPS) < 0) {
> + retval = -errno;
> + do_perror("capng_apply");
> + goto err_suid;
> }
> return 0;
>