virt-login-shell: fix regressions in behavior

Our fixes for CVE-2013-4400 were so effective at "fixing" bugs
in virt-login-shell that we ended up fixing it into a useless
do-nothing program.

Commit 3e2f27e1 picked the name LIBVIRT_SETUID_RPC_CLIENT for
the witness macro when we are doing secure compilation.  But
commit 9cd6a57d checked whether the name IN_VIRT_LOGIN_SHELL,
from an earlier version of the patch series, was defined; with
the net result that virt-login-shell invariably detected that
it was setuid and failed virInitialize.

Commit b7fcc799 closed all fds larger than stderr, but in the
wrong place.  Looking at the larger context, we mistakenly did
the close in between obtaining the set of namespace fds, then
actually using those fds to switch namespace, which means that
virt-login-shell will ALWAYS fail.

This is the minimal patch to fix the regressions, although
further patches are also worth having to clean up poor
semantics of the resulting program (for example, it is rude to
not pass on the exit status of the wrapped program back to the
invoking shell).

* tools/virt-login-shell.c (main): Don't close fds until after
namespace swap.
* src/libvirt.c (virGlobalInit): Use correct macro.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/src/libvirt.c b/src/libvirt.c
index c6d8a25501548e9c7d918320cec71173ecf3ebbe..f287933be263e1cf32b2799e11d33265357e313c 100644 (file)
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -357,7 +357,7 @@ virGlobalInit(void)
         virErrorInitialize() < 0)
         goto error;
 
-#ifndef IN_VIRT_LOGIN_SHELL
+#ifndef LIBVIRT_SETUID_RPC_CLIENT
     if (virIsSUID()) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("libvirt.so is not safe to use from setuid programs"));

diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index 8b35e34384304ba5ab1205185ae7698064ba74d1..c0a4eeb050fdc28aeb63b2914f8553a58edea5ac 100644 (file)
--- a/tools/virt-login-shell.c
+++ b/tools/virt-login-shell.c
@@ -1,7 +1,7 @@
 /*
  * virt-login-shell.c: a shell to connect to a container
  *
- * Copyright (C) 2013 Red Hat, Inc.
+ * Copyright (C) 2013-2014 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -317,15 +317,6 @@ main(int argc, char **argv)
 
         int openmax = sysconf(_SC_OPEN_MAX);
         int fd;
-        if (openmax < 0) {
-            virReportSystemError(errno,  "%s",
-                                 _("sysconf(_SC_OPEN_MAX) failed"));
-            return EXIT_FAILURE;
-        }
-        for (fd = 3; fd < openmax; fd++) {
-            int tmpfd = fd;
-            VIR_MASS_CLOSE(tmpfd);
-        }
 
         /* Fork once because we don't want to affect
          * virt-login-shell's namespace itself
@@ -354,6 +345,16 @@ main(int argc, char **argv)
         if (ret < 0)
             return EXIT_FAILURE;
 
+        if (openmax < 0) {
+            virReportSystemError(errno,  "%s",
+                                 _("sysconf(_SC_OPEN_MAX) failed"));
+            return EXIT_FAILURE;
+        }
+        for (fd = 3; fd < openmax; fd++) {
+            int tmpfd = fd;
+            VIR_MASS_CLOSE(tmpfd);
+        }
+
         if (virFork(&ccpid) < 0)
             return EXIT_FAILURE;