Richard W.M. Jones
2010-Mar-19 17:34 UTC
[Libguestfs] [PATCH FOR DISCUSSION ONLY] Allow setting of user:group for qemu subprocess.
This is an experimental (not working) patch which would allow you to set the user and group of the qemu process, assuming the main program is running as root. The reason for this is to allow access as root to disk images which are located on "root-squashed" NFS volumes. This is a particular concern for virt-v2v. The most immediate problem with the patch (which can be fixed easily) is that the non-root qemu cannot access the appliance: qemu: could not load kernel '/tmp/libguestfsj2CItc/kernel': Permission denied In terms of the bigger picture I'm not convinced that this patch is really going to be useful. Firstly various commands currently try to access the disk image from the main process (notably guestfs_add_drive). Secondly any serious program using libguestfs will want to touch the disk image elsewhere, so the root-squashing problem will have to be tackled there too. It sounds as if for virt-v2v the whole program should just setuid itself to a non-root user as early as possible, rather than pushing this into libguestfs. On the other hand, not running qemu as root even when libguestfs itself is root, is an appealing idea if the permissions issues could be resolved. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v -------------- next part -------------->From ef05811e8610fa2cf8db29cea0f7013913a9ad46 Mon Sep 17 00:00:00 2001From: Richard Jones <rjones at redhat.com> Date: Fri, 19 Mar 2010 17:21:58 +0000 Subject: [PATCH] Allow setting of user:group for qemu subprocess. Allow the user and group of the qemu subprocess to be set. If the main process is running as root, then this causes the qemu subprocess to change user and/or group to the user:group specified. In theory this should permit access to disk images which are located on "root-squashed" NFS volumes. Note that the host still touches the file (as root) so your mileage may vary. For example, 'guestfs_add_drive' will check the file exists (using access (F_OK)). And guestfish has various commands for creating blank disk images which might still fail. --- configure.ac | 5 ++- src/generator.ml | 53 +++++++++++++++++++++++++++++++ src/guestfs.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index aa757dc..76cbc56 100644 --- a/configure.ac +++ b/configure.ac @@ -129,7 +129,10 @@ dnl Check sizeof long. AC_CHECK_SIZEOF([long]) dnl Headers. -AC_CHECK_HEADERS([errno.h sys/types.h sys/un.h sys/wait.h sys/socket.h endian.h byteswap.h]) +AC_CHECK_HEADERS([errno.h sys/types.h sys/un.h sys/wait.h sys/socket.h endian.h byteswap.h pwd.h grp.h]) + +dnl Functions. +AC_CHECK_FUNCS([getpwnam getgrnam getgroups setuid setgid]) dnl Check for rpcgen and XDR library. rpcgen is optional. AC_CHECK_PROG([RPCGEN],[rpcgen],[rpcgen],[no]) diff --git a/src/generator.ml b/src/generator.ml index 83f307b..dd2c8b1 100755 --- a/src/generator.ml +++ b/src/generator.ml @@ -922,6 +922,59 @@ to specify the QEMU interface emulation to use at run time."); This is the same as C<guestfs_add_drive_ro> but it allows you to specify the QEMU interface emulation to use at run time."); + ("set_user", (RErr, [OptString "user"]), -1, [FishAlias "user"], + [], + "set the optional user for running qemu", + "\ +This sets the optional user for running the qemu +subprocess. If this is defined for a handle, and if the +parent process is running as root, then this causes the +subprocess (qemu) to be run as the named user. + +The only real use for this is when the disk image is located +on a \"root-squashed\" NFS volume. A root-owned processes +might not be able to access the disk image, whereas by just +changing the UID, that access might be allowed. (This tells you +more about the stupidity of NFS than anything else). + +The user specified must have a non-zero UID. + +See also C<guestfs_get_user>, C<guestfs_set_group> and +C<guestfs_get_group>. + +For more information on the architecture of libguestfs, +see L<guestfs(3)>."); + + ("get_user", (RConstOptString "user", []), -1, [], + [], + "return the optional user for running qemu", + "\ +This returns the optional user for running qemu. +See C<guestfs_set_user>."); + + ("set_group", (RErr, [OptString "group"]), -1, [FishAlias "group"], + [], + "set the optional group for running qemu", + "\ +This sets the optional group for running the qemu +subprocess. If this is defined for a handle, and if the +parent process is running as root, then this causes the +subprocess (qemu) to be run as the named group. + +The group specified must have a non-zero GID. + +See also C<guestfs_set_user> and C<guestfs_get_group>. + +For more information on the architecture of libguestfs, +see L<guestfs(3)>."); + + ("get_group", (RConstOptString "group", []), -1, [], + [], + "return the optional group for running qemu", + "\ +This returns the optional group for running qemu. +See C<guestfs_set_group>."); + ] (* daemon_functions are any functions which cause some action diff --git a/src/guestfs.c b/src/guestfs.c index 729b687..475e1dd 100644 --- a/src/guestfs.c +++ b/src/guestfs.c @@ -34,6 +34,14 @@ #include <sys/select.h> #include <dirent.h> +#ifdef HAVE_PWD_H +#include <pwd.h> +#endif + +#ifdef HAVE_GRP_H +#include <grp.h> +#endif + #include <rpc/types.h> #include <rpc/xdr.h> @@ -137,6 +145,8 @@ struct guestfs_h int selinux; /* selinux enabled? */ + char *user, *group; /* Setuid to this user:group (if not NULL). */ + char *last_error; /* Callbacks. */ @@ -687,6 +697,36 @@ guestfs__get_recovery_proc (guestfs_h *g) return g->recovery_proc; } +int +guestfs__set_user (guestfs_h *g, const char *user) +{ + free (g->user); + + g->user = user ? safe_strdup (g, user) : NULL; + return 0; +} + +const char * +guestfs__get_user (guestfs_h *g) +{ + return g->user; +} + +int +guestfs__set_group (guestfs_h *g, const char *group) +{ + free (g->group); + + g->group = group ? safe_strdup (g, group) : NULL; + return 0; +} + +const char * +guestfs__get_group (guestfs_h *g) +{ + return g->group; +} + /* Add a string to the current command line. */ static void incr_cmdline_size (guestfs_h *g) @@ -1284,6 +1324,58 @@ guestfs__launch (guestfs_h *g) setpgid (0, 0); #endif + /* Setgid/setuid if asked. */ + if (g->group) { +#if defined(HAVE_GRP_H) && defined(HAVE_SETGID) + struct group *grp = getgrnam (g->group); + if (grp == NULL) { + fprintf (stderr, _("libguestfs: %s: %m\n"), g->group); + _exit (EXIT_FAILURE); + } + if (setgid (grp->gr_gid) == -1) { + fprintf (stderr, _("libguestfs: setgid: %s (gid %d): %m\n"), + g->group, grp->gr_gid); + _exit (EXIT_FAILURE); + } + +#ifdef HAVE_SETGROUPS + /* Process might have extra groups, discard them. */ + if (setgroups (1, &gid) == -1) { + fprintf (stderr, _("libguestfs: setgroups: gid %d: %m\n"), + grp->gr_gid); + _exit (EXIT_FAILURE); + } +#endif +#else /* !HAVE_GRP_H || !HAVE_SETGID */ + fprintf (stderr, _("libguestfs: 'guestfs_set_group' called, but this system does not support setting the group\n")); + _exit (EXIT_FAILURE); +#endif + } + + if (g->user) { +#if defined(HAVE_PWD_H) && defined(HAVE_SETUID) + struct passwd *pwd = getpwnam (g->user); + if (pwd == NULL) { + fprintf (stderr, _("libguestfs: %s: %m\n"), g->user); + _exit (EXIT_FAILURE); + } + if (setuid (pwd->pw_uid) == -1) { + fprintf (stderr, _("libguestfs: setuid: %s (uid %d): %m\n"), + g->user, pwd->pw_uid); + _exit (EXIT_FAILURE); + } +#else /* !HAVE_PWD_H || !HAVE_SETUID */ + fprintf (stderr, _("libguestfs: 'guestfs_set_user' called, but this system does not support setting the user\n")); + _exit (EXIT_FAILURE); +#endif + } + + if ((g->user && (getuid () == 0 || geteuid () == 0)) || + (g->group && (getgid () == 0 || getegid () == 0))) { + fprintf (stderr, _("libguestfs: still have root privileges after trying to discard them, refusing to continue\n")); + _exit (EXIT_FAILURE); + } + setenv ("LC_ALL", "C", 1); execv (g->qemu, g->cmdline); /* Run qemu. */ -- 1.6.5.2
Reasonably Related Threads
- [PATCH for discussion only] lib: libvirt: If root, run qemu subprocess as root.root.
- [PATCH] lib: Limit space and time used by 'qemu-img info' subprocess.
- ANNOUNCE: libguestfs 1.32 released
- [cfe-dev] [lldb-dev] GitHub anyone?
- [PATCH] p2v: wait for qemu-nbd before starting conversion (RHBZ#1167774)