[PATCH] virtfs-proxy-helper: switch from libcap to libcap-ng

Paolo Bonzini posted 1 patch 4 years, 4 months ago
Test asan passed
Test checkpatch passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191129111632.22840-2-pbonzini@redhat.com
Maintainers: Greg Kurz <groug@kaod.org>
configure                   |  18 +------
fsdev/virtfs-proxy-helper.c | 100 ++++++++++++++++--------------------
2 files changed, 46 insertions(+), 72 deletions(-)
[PATCH] virtfs-proxy-helper: switch from libcap to libcap-ng
Posted by Paolo Bonzini 4 years, 4 months ago
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 <pbonzini@redhat.com>
---
 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;
 
-- 
2.21.0


Re: [PATCH] virtfs-proxy-helper: switch from libcap to libcap-ng
Posted by Greg Kurz 4 years, 4 months ago
On Fri, 29 Nov 2019 12:16:32 +0100
Paolo Bonzini <pbonzini@redhat.com> 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 <pbonzini@redhat.com>
> ---

Nice. :)

Reviewed-by: Greg Kurz <groug@kaod.org>

Paolo,

I can take this through my 9p tree if you want. Otherwise,

Acked-by: Greg Kurz <groug@kaod.org>



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 <thuth@redhat.com>
Date:   Thu Sep 5 13:33:46 2019 +0200

    travis.yml: Install libcap-dev for testing virito-9p

and

commit e7dc804ef0d7cac9ac8b4a1324ab7dbfafb55704
Author: Thomas Huth <thuth@redhat.com>
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;
>  


Re: [PATCH] virtfs-proxy-helper: switch from libcap to libcap-ng
Posted by Paolo Bonzini 4 years, 4 months ago
On 29/11/19 13:32, Greg Kurz wrote:
> Nice. :)
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> Paolo,
> 
> I can take this through my 9p tree if you want. Otherwise,
> 
> Acked-by: Greg Kurz <groug@kaod.org>

Yes, please do it since it's self-contained.  You'd probably also test
it better than me. :)

Paolo


Re: [PATCH] virtfs-proxy-helper: switch from libcap to libcap-ng
Posted by Greg Kurz 4 years, 4 months ago
On Fri, 29 Nov 2019 13:49:20 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 29/11/19 13:32, Greg Kurz wrote:
> > Nice. :)
> > 
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > 
> > Paolo,
> > 
> > I can take this through my 9p tree if you want. Otherwise,
> > 
> > Acked-by: Greg Kurz <groug@kaod.org>
> 
> Yes, please do it since it's self-contained.  You'd probably also test
> it better than me. :)
> 
> Paolo
> 

Ok I'll just do that then.

Cheers,

--
Greg

Re: [PATCH] virtfs-proxy-helper: switch from libcap to libcap-ng
Posted by Greg Kurz 4 years, 3 months ago
On Fri, 29 Nov 2019 13:59:37 +0100
Greg Kurz <groug@kaod.org> wrote:

> On Fri, 29 Nov 2019 13:49:20 +0100
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> > On 29/11/19 13:32, Greg Kurz wrote:
> > > Nice. :)
> > > 
> > > Reviewed-by: Greg Kurz <groug@kaod.org>
> > > 
> > > Paolo,
> > > 
> > > I can take this through my 9p tree if you want. Otherwise,
> > > 
> > > Acked-by: Greg Kurz <groug@kaod.org>
> > 
> > Yes, please do it since it's self-contained.  You'd probably also test
> > it better than me. :)
> > 
> > Paolo
> > 
> 
> Ok I'll just do that then.
> 

And it happens to be missing an extra-change in Makefile :)

-fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
+fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap-ng

I've fixed this up in my tree.

> Cheers,
> 
> --
> Greg


Re: [PATCH] virtfs-proxy-helper: switch from libcap to libcap-ng
Posted by Paolo Bonzini 4 years, 3 months ago
On 03/12/19 11:34, Greg Kurz wrote:
> On Fri, 29 Nov 2019 13:59:37 +0100
> Greg Kurz <groug@kaod.org> wrote:
> 
>> On Fri, 29 Nov 2019 13:49:20 +0100
>> Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>> On 29/11/19 13:32, Greg Kurz wrote:
>>>> Nice. :)
>>>>
>>>> Reviewed-by: Greg Kurz <groug@kaod.org>
>>>>
>>>> Paolo,
>>>>
>>>> I can take this through my 9p tree if you want. Otherwise,
>>>>
>>>> Acked-by: Greg Kurz <groug@kaod.org>
>>>
>>> Yes, please do it since it's self-contained.  You'd probably also test
>>> it better than me. :)
>>>
>>> Paolo
>>>
>>
>> Ok I'll just do that then.
>>
> 
> And it happens to be missing an extra-change in Makefile :)
> 
> -fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
> +fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap-ng

The new line is not needed, -lcap-ng should already be in LIBS via

LIBS+=-lz $(LIBS_TOOLS)

However, removing -lcap is certainly a good idea. :)

Paolo


Re: [PATCH] virtfs-proxy-helper: switch from libcap to libcap-ng
Posted by Daniel P. Berrangé 4 years, 3 months ago
On Fri, Nov 29, 2019 at 12:16:32PM +0100, Paolo Bonzini 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 <pbonzini@redhat.com>
> ---
>  configure                   |  18 +------
>  fsdev/virtfs-proxy-helper.c | 100 ++++++++++++++++--------------------
>  2 files changed, 46 insertions(+), 72 deletions(-)

The dockerfiles can also be updated to remove libcap-dev/devel

> 
> 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;
>  
> -- 
> 2.21.0
> 

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


Re: [PATCH] virtfs-proxy-helper: switch from libcap to libcap-ng
Posted by Daniel P. Berrangé 4 years, 4 months ago
On Fri, Nov 29, 2019 at 12:16:32PM +0100, Paolo Bonzini 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 <pbonzini@redhat.com>
> ---
>  configure                   |  18 +------
>  fsdev/virtfs-proxy-helper.c | 100 ++++++++++++++++--------------------
>  2 files changed, 46 insertions(+), 72 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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