From e879d4e45082bb2d43c45704b7d9592eebcc09b4 Mon Sep 17 00:00:00 2001

Message-Id: <e879d4e45082bb2d43c45704b7d9592eebcc09b4.1390394206.git.jdenemar@redhat.com>

From: Eric Blake <eblake@redhat.com>

Date: Fri, 17 Jan 2014 10:35:01 -0700

Subject: [PATCH] virt-login-shell: fix regressions in behavior

https://bugzilla.redhat.com/show_bug.cgi?id=1015247

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>

(cherry picked from commit 3d007cb5f892bb9fd3f6efac6dc89a8e00e94922)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/libvirt.c            |  2 +-

 tools/virt-login-shell.c | 21 +++++++++++----------

 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c

index 31600e8..61cae03 100644

--- a/src/libvirt.c

+++ b/src/libvirt.c

@@ -409,7 +409,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 baa6202..a9a406c 100644

--- 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

@@ -312,15 +312,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

@@ -349,6 +340,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;

-- 

1.8.5.3