Call febootstrap-supermin-helper using the new -u and -g options

Use febootstrap-supermin-helper's new -u and -g command line options to setuid,
rather than doing it in libguestfs.

This resolves an issue with the generation of the cached appliance checksum. The
checksum was being generated by a call to febootstrap-supermin-helper through
popen(). Unfortunately, a bash misfeature meant that euid would be reset to uid,
and the checksum was generated for uid, not euid. When virt-v2v is writing to a
RHEV target, uid == 0 and euid == 36, which resulted in a cached appliance being
created for root with permissions for uid 36.

Note this requires febootstrap 2.10.

diff --git a/README b/README
index e231840..a88f4cc 100644 (file)
--- a/README
+++ b/README
@@ -39,7 +39,7 @@ Requirements
 
 - recent QEMU >= 0.12 with virtio-serial support
 
-- febootstrap >= 2.9
+- febootstrap >= 2.10
 
 - fakeroot
 

diff --git a/src/appliance.c b/src/appliance.c
index a4f5f6f..8c8ddd0 100644 (file)
--- a/src/appliance.c
+++ b/src/appliance.c
@@ -159,13 +159,27 @@ calculate_supermin_checksum (guestfs_h *g, const char *supermin_path)
 {
   size_t len = 2 * strlen (supermin_path) + 256;
   char cmd[len];
-  snprintf (cmd, len,
-            "febootstrap-supermin-helper%s "
-            "-f checksum "
-            "'%s/supermin.d' "
-            host_cpu,
-            g->verbose ? " --verbose" : "",
-            supermin_path);
+  int pass_u_g_args = getuid () != geteuid () || getgid () != getegid ();
+
+  if (!pass_u_g_args)
+    snprintf (cmd, len,
+              "febootstrap-supermin-helper%s "
+              "-f checksum "
+              "'%s/supermin.d' "
+              host_cpu,
+              g->verbose ? " --verbose" : "",
+              supermin_path);
+  else
+    snprintf (cmd, len,
+              "febootstrap-supermin-helper%s "
+              "-u %i "
+              "-g %i "
+              "-f checksum "
+              "'%s/supermin.d' "
+              host_cpu,
+              g->verbose ? " --verbose" : "",
+              geteuid (), getegid (),
+              supermin_path);
 
   if (g->verbose)
     guestfs___print_timestamped_message (g, "%s", cmd);
@@ -361,11 +375,7 @@ build_supermin_appliance (guestfs_h *g,
 }
 
 /* Run febootstrap-supermin-helper and tell it to generate the
- * appliance.  Note that we have to do an explicit fork/exec here.
- * 'system' goes via the shell, and on systems that have bash, bash
- * has a misfeature where it resets the euid to uid which breaks
- * virt-v2v.  'posix_spawn' was also considered but that doesn't allow
- * us to reset the umask.
+ * appliance.
  */
 static int
 run_supermin_helper (guestfs_h *g, const char *supermin_path,
@@ -376,6 +386,10 @@ run_supermin_helper (guestfs_h *g, const char *supermin_path,
   const char *argv[30];
   size_t i = 0;
 
+  char uid[32];
+  snprintf (uid, sizeof uid, "%i", geteuid ());
+  char gid[32];
+  snprintf (gid, sizeof gid, "%i", getegid ());
   char supermin_d[pathlen + 32];
   snprintf (supermin_d, pathlen + 32, "%s/supermin.d", supermin_path);
   char kernel[cdlen + 32];
@@ -385,9 +399,17 @@ run_supermin_helper (guestfs_h *g, const char *supermin_path,
   char root[cdlen + 32];
   snprintf (root, cdlen + 32, "%s/root", cachedir);
 
+  int pass_u_g_args = getuid () != geteuid () || getgid () != getegid ();
+
   argv[i++] = "febootstrap-supermin-helper";
   if (g->verbose)
     argv[i++] = "--verbose";
+  if (pass_u_g_args) {
+    argv[i++] = "-u";
+    argv[i++] = uid;
+    argv[i++] = "-g";
+    argv[i++] = gid;
+  }
   argv[i++] = "-f";
   argv[i++] = "ext2";
   argv[i++] = supermin_d;
@@ -426,31 +448,6 @@ run_supermin_helper (guestfs_h *g, const char *supermin_path,
    */
   umask (0022);
 
-  /* Set uid/gid in the child.  This is a workaround for a misfeature
-   * in bash which breaks virt-v2v - see the comment at the top of
-   * this function.
-   */
-  if (getuid () == 0) {
-    int egid = getegid ();
-    int euid = geteuid ();
-
-    if (egid != 0 || euid != 0) {
-      if (seteuid (0) == -1) {
-        perror ("seteuid");
-        _exit (EXIT_FAILURE);
-      }
-
-      if (setgid (egid) == -1) {
-        perror ("setgid");
-        _exit (EXIT_FAILURE);
-      }
-
-      if (setuid (euid) == -1) {
-        perror ("setuid");
-        _exit (EXIT_FAILURE);
-      }
-    }
-  }
   execvp ("febootstrap-supermin-helper", (char * const *) argv);
   perror ("execvp");
   _exit (EXIT_FAILURE);