[Libguestfs] [PATCH] Fix error launching libguestfs when euid != uid

[Matthew Booth]

When writing to a RHEV target, virt-v2v launches the libguestfs appliance with
euid:egid = 36:36, which is required to write to an NFS target using
root_squash. Since the update to use an febootstrap cached appliance, this
causes an error on startup as the cached files are owned by root, but the cache
directory is owned by 36:36. The reason for this is that execve() resets euid
and egid to uid and gid respectively, so when febootstrap-supermin-helper is
executed, it runs as root:root. The cache directory, however, is created by
libguestfs directly without exec()ing another program, so it has the correct
ownership.

This patch fixes this issue by setting real uid and gid to euid and egid
respectively before exec()ing the helper program.
---
 src/appliance.c |  134 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 125 insertions(+), 9 deletions(-)

diff --git a/src/appliance.c b/src/appliance.c
index 3c3279b..be5e167 100644
--- a/src/appliance.c
+++ b/src/appliance.c
@@ -27,6 +27,7 @@
 #include <time.h>
 #include <sys/stat.h>
 #include <sys/select.h>
+#include <sys/wait.h>
 #include <utime.h>
 
 #ifdef HAVE_SYS_TYPES_H
@@ -49,6 +50,7 @@ static int contains_ordinary_appliance (guestfs_h *g, const char *path, void *da
 static char *calculate_supermin_checksum (guestfs_h *g, const char *supermin_path);
 static int check_for_cached_appliance (guestfs_h *g, const char *supermin_path, const char *checksum, char **kernel, char **initrd, char **appliance);
 static int build_supermin_appliance (guestfs_h *g, const char *supermin_path, const char *checksum, char **kernel, char **initrd, char **appliance);
+static pid_t fork_helper(guestfs_h *g);
 
 /* Locate or build the appliance.
  *
@@ -170,19 +172,59 @@ calculate_supermin_checksum (guestfs_h *g, const char *supermin_path)
   if (g->verbose)
     guestfs___print_timestamped_message (g, "%s", cmd);
 
-  /* Errors here are non-fatal, so we don't need to call error(). */
-  FILE *pp = popen (cmd, "r");
-  if (pp == NULL)
+  int fds[2];
+  if (pipe(fds) == -1) {
+    error (g, _("pipe failed: %m"));
     return NULL;
+  }
+ 
+  int pid = fork_helper(g);
+  if (pid == -1) {
+    return NULL;
+  }
+ 
+  /* exec febootstrap-supermin-helper in the child */
+  if (pid == 0) {
+    /* Close the read end */
+    if(close(fds[0]) == -1) {
+      perror("close read end");
+      exit(1);
+    }
+ 
+    /* dup2 the write end to stdout */
+    if(dup2(fds[1], STDOUT_FILENO) == -1) {
+      perror("dup2");
+      exit(1);
+    }
+ 
+    /* Close the write end */
+    if(close(fds[1]) == -1) {
+      perror("close write end");
+      exit(1);
+    }
+ 
+    char *const argv[] = { strdup("/bin/sh"), strdup("-c"), cmd, NULL };
+    if (execv("/bin/sh", argv) == -1) {
+      perror("execv");
+      exit(1);
+    }
+  }
+
+  FILE *pp = fdopen(fds[0], "r");
+  if (pp == NULL) {
+    perror("fdopen");
+    goto helper_error;
+  }
 
   char checksum[256];
   if (fgets (checksum, sizeof checksum, pp) == NULL) {
-    pclose (pp);
-    return NULL;
+    fclose (pp);
+    goto helper_error;
   }
 
-  if (pclose (pp) == -1) {
-    perror ("pclose");
+  fclose (pp);
+  if (waitpid(pid, NULL, 0) == -1) {
+    perror ("waitpid");
     return NULL;
   }
 
@@ -197,6 +239,18 @@ calculate_supermin_checksum (guestfs_h *g, const char *supermin_path)
     checksum[--len] = '\0';
 
   return safe_strndup (g, checksum, len);
+
+  int dummy[128];
+helper_error:
+  /* Consume any remaining output, ignoring errors */
+  while (read(fds[0], dummy, sizeof(dummy) > 0))
+    ;
+
+  if (waitpid(pid, NULL, 0) == -1) {
+      perror("reaping feboostrap-supermin-helper");
+  }
+
+  return NULL;
 }
 
 /* Check for cached appliance in $TMPDIR/$checksum.  Check it exists
@@ -357,8 +411,28 @@ build_supermin_appliance (guestfs_h *g,
             cachedir, cachedir, cachedir);
   if (g->verbose)
     guestfs___print_timestamped_message (g, "%s", cmd);
-  int r = system (cmd);
-  if (r == -1 || WEXITSTATUS (r) != 0) {
+
+  pid_t pid = fork_helper(g);
+  if (pid == -1) {
+    return -1;
+  }
+
+  /* exec febootstrap-supermin-helper in the child */
+  if (pid == 0) {
+    char * const argv[] = { strdup("/bin/sh"), strdup("-c"), cmd, NULL };
+    if (execv("/bin/sh", argv) == -1) {
+      perror("execv");
+      exit(1);
+    }
+  }
+
+  int r;
+  if (waitpid(pid, &r, 0) == -1) {
+    error (g, _("error waiting for command: %s (%m)"), cmd);
+    return -1;
+  }
+
+  if (WEXITSTATUS (r) != 0) {
     error (g, _("external command failed: %s"), cmd);
     return -1;
   }
@@ -463,3 +537,45 @@ dir_contains_files (const char *dir, ...)
   va_end (args);
   return 1;
 }
+
+/* Launch may be called by a seteuid/setegid process (virt-v2v does this).
+ * Unfortunately, execve resets EGID/EUID to GID/UID. This means that files
+ * created by a subprocess will have the wrong ownership. To work round this,
+ * we set the real GID/UID first before exec. */
+static pid_t fork_helper(guestfs_h *g)
+{
+  pid_t pid = fork();
+  if (pid == -1) {
+    error (g, _("fork failed: %m"));
+    return -1;
+  }
+
+  /* Set euid/egid in the child.
+   * Note that the child just needs to exit with an error, not return
+   */
+  if (pid == 0) {
+    if (getuid() == 0) {
+      int egid = getegid();
+      int euid = geteuid();
+
+      if (egid != 0 || euid != 0) {
+        if (seteuid(0) == -1) {
+          perror("seteuid");
+          exit(1);
+        }
+
+        if (setgid(egid) == -1) {
+          perror("setgid");
+          exit(1);
+        }
+
+        if (setuid(euid) == -1) {
+          perror("setuid");
+          exit(1);
+        }
+      }
+    }
+  }
+
+  return pid;
+}
-- 
1.7.2.3