[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[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
>From ef05811e8610fa2cf8db29cea0f7013913a9ad46 Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]