configure | 18 +------ fsdev/virtfs-proxy-helper.c | 100 ++++++++++++++++-------------------- 2 files changed, 46 insertions(+), 72 deletions(-)
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
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; >
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
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
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
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
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 :|
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 :|
© 2016 - 2024 Red Hat, Inc.