render / rpms / libvirt

Forked from rpms/libvirt 10 months ago
Clone
43fe83
From 4799286cbbf12b5d2734ae79d0580d92b8d3b100 Mon Sep 17 00:00:00 2001
43fe83
Message-Id: <4799286cbbf12b5d2734ae79d0580d92b8d3b100.1383321465.git.jdenemar@redhat.com>
43fe83
From: "Daniel P. Berrange" <berrange@redhat.com>
43fe83
Date: Wed, 30 Oct 2013 17:01:50 +0000
43fe83
Subject: [PATCH] Make virCommand env handling robust in setuid env
43fe83
43fe83
For
43fe83
43fe83
  https://bugzilla.redhat.com/show_bug.cgi?id=1015247
43fe83
43fe83
When running setuid, we must be careful about what env vars
43fe83
we allow commands to inherit from us. Replace the
43fe83
virCommandAddEnvPass function with two new ones which do
43fe83
filtering
43fe83
43fe83
  virCommandAddEnvPassAllowSUID
43fe83
  virCommandAddEnvPassBlockSUID
43fe83
43fe83
And make virCommandAddEnvPassCommon use the appropriate
43fe83
ones
43fe83
43fe83
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
43fe83
(cherry picked from commit 9b8f307c6ad002a17a0510513883d06395636793)
43fe83
43fe83
Conflicts:
43fe83
	src/qemu/qemu_command.c
43fe83
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
43fe83
---
43fe83
 src/libvirt_private.syms |  3 ++-
43fe83
 src/lxc/lxc_process.c    |  2 +-
43fe83
 src/qemu/qemu_command.c  |  6 +++---
43fe83
 src/rpc/virnetsocket.c   | 16 ++++++++--------
43fe83
 src/util/vircommand.c    | 50 +++++++++++++++++++++++++++++++++++++-----------
43fe83
 src/util/vircommand.h    |  8 ++++++--
43fe83
 tests/commandtest.c      |  8 ++++----
43fe83
 7 files changed, 63 insertions(+), 30 deletions(-)
43fe83
43fe83
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
43fe83
index 374a332..ca6275c 100644
43fe83
--- a/src/libvirt_private.syms
43fe83
+++ b/src/libvirt_private.syms
43fe83
@@ -1237,7 +1237,8 @@ virCommandAddArgSet;
43fe83
 virCommandAddEnvBuffer;
43fe83
 virCommandAddEnvFormat;
43fe83
 virCommandAddEnvPair;
43fe83
-virCommandAddEnvPass;
43fe83
+virCommandAddEnvPassAllowSUID;
43fe83
+virCommandAddEnvPassBlockSUID;
43fe83
 virCommandAddEnvPassCommon;
43fe83
 virCommandAddEnvString;
43fe83
 virCommandAllowCap;
43fe83
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
43fe83
index a5f1f35..a88fb4d 100644
43fe83
--- a/src/lxc/lxc_process.c
43fe83
+++ b/src/lxc/lxc_process.c
43fe83
@@ -740,7 +740,7 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
43fe83
     cmd = virCommandNew(vm->def->emulator);
43fe83
 
43fe83
     /* The controller may call ip command, so we have to retain PATH. */
43fe83
-    virCommandAddEnvPass(cmd, "PATH");
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "PATH", "/bin:/usr/bin");
43fe83
 
43fe83
     virCommandAddEnvFormat(cmd, "LIBVIRT_DEBUG=%d",
43fe83
                            virLogGetDefaultPriority());
43fe83
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
43fe83
index 78f07c8..0292b72 100644
43fe83
--- a/src/qemu/qemu_command.c
43fe83
+++ b/src/qemu/qemu_command.c
43fe83
@@ -6997,7 +6997,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
43fe83
      * security issues and might not work when using VNC.
43fe83
      */
43fe83
     if (cfg->vncAllowHostAudio)
43fe83
-        virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
43fe83
+        virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
43fe83
     else
43fe83
         virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
43fe83
 
43fe83
@@ -7242,8 +7242,8 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
43fe83
          * use QEMU's host audio drivers, possibly SDL too
43fe83
          * User can set these two before starting libvirtd
43fe83
          */
43fe83
-        virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
43fe83
-        virCommandAddEnvPass(cmd, "SDL_AUDIODRIVER");
43fe83
+        virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
43fe83
+        virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL);
43fe83
 
43fe83
         /* New QEMU has this flag to let us explicitly ask for
43fe83
          * SDL graphics. This is better than relying on the
43fe83
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
43fe83
index ae81512..fcd41ca 100644
43fe83
--- a/src/rpc/virnetsocket.c
43fe83
+++ b/src/rpc/virnetsocket.c
43fe83
@@ -127,9 +127,9 @@ static int virNetSocketForkDaemon(const char *binary)
43fe83
                                              NULL);
43fe83
 
43fe83
     virCommandAddEnvPassCommon(cmd);
43fe83
-    virCommandAddEnvPass(cmd, "XDG_CACHE_HOME");
43fe83
-    virCommandAddEnvPass(cmd, "XDG_CONFIG_HOME");
43fe83
-    virCommandAddEnvPass(cmd, "XDG_RUNTIME_DIR");
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "XDG_CACHE_HOME", NULL);
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "XDG_CONFIG_HOME", NULL);
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "XDG_RUNTIME_DIR", NULL);
43fe83
     virCommandClearCaps(cmd);
43fe83
     virCommandDaemonize(cmd);
43fe83
     ret = virCommandRun(cmd, NULL);
43fe83
@@ -680,11 +680,11 @@ int virNetSocketNewConnectSSH(const char *nodename,
43fe83
 
43fe83
     cmd = virCommandNew(binary ? binary : "ssh");
43fe83
     virCommandAddEnvPassCommon(cmd);
43fe83
-    virCommandAddEnvPass(cmd, "KRB5CCNAME");
43fe83
-    virCommandAddEnvPass(cmd, "SSH_AUTH_SOCK");
43fe83
-    virCommandAddEnvPass(cmd, "SSH_ASKPASS");
43fe83
-    virCommandAddEnvPass(cmd, "DISPLAY");
43fe83
-    virCommandAddEnvPass(cmd, "XAUTHORITY");
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "KRB5CCNAME", NULL);
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "SSH_AUTH_SOCK", NULL);
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "SSH_ASKPASS", NULL);
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL);
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "XAUTHORITY", NULL);
43fe83
     virCommandClearCaps(cmd);
43fe83
 
43fe83
     if (service)
43fe83
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
43fe83
index 00ff69a..fca0e09 100644
43fe83
--- a/src/util/vircommand.c
43fe83
+++ b/src/util/vircommand.c
43fe83
@@ -1247,21 +1247,49 @@ virCommandAddEnvBuffer(virCommandPtr cmd, virBufferPtr buf)
43fe83
 
43fe83
 
43fe83
 /**
43fe83
- * virCommandAddEnvPass:
43fe83
+ * virCommandAddEnvPassAllowSUID:
43fe83
  * @cmd: the command to modify
43fe83
  * @name: the name to look up in current environment
43fe83
  *
43fe83
  * Pass an environment variable to the child
43fe83
  * using current process' value
43fe83
+ *
43fe83
+ * Allow to be passed even if setuid
43fe83
+ */
43fe83
+void
43fe83
+virCommandAddEnvPassAllowSUID(virCommandPtr cmd, const char *name)
43fe83
+{
43fe83
+    const char *value;
43fe83
+    if (!cmd || cmd->has_error)
43fe83
+        return;
43fe83
+
43fe83
+    value = virGetEnvAllowSUID(name);
43fe83
+    if (value)
43fe83
+        virCommandAddEnvPair(cmd, name, value);
43fe83
+}
43fe83
+
43fe83
+
43fe83
+/**
43fe83
+ * virCommandAddEnvPassBlockSUID:
43fe83
+ * @cmd: the command to modify
43fe83
+ * @name: the name to look up in current environment
43fe83
+ * @defvalue: value to return if running setuid, may be NULL
43fe83
+ *
43fe83
+ * Pass an environment variable to the child
43fe83
+ * using current process' value.
43fe83
+ *
43fe83
+ * Do not pass if running setuid
43fe83
  */
43fe83
 void
43fe83
-virCommandAddEnvPass(virCommandPtr cmd, const char *name)
43fe83
+virCommandAddEnvPassBlockSUID(virCommandPtr cmd, const char *name, const char *defvalue)
43fe83
 {
43fe83
-    char *value;
43fe83
+    const char *value;
43fe83
     if (!cmd || cmd->has_error)
43fe83
         return;
43fe83
 
43fe83
-    value = getenv(name);
43fe83
+    value = virGetEnvBlockSUID(name);
43fe83
+    if (!value)
43fe83
+        value = defvalue;
43fe83
     if (value)
43fe83
         virCommandAddEnvPair(cmd, name, value);
43fe83
 }
43fe83
@@ -1286,13 +1314,13 @@ virCommandAddEnvPassCommon(virCommandPtr cmd)
43fe83
 
43fe83
     virCommandAddEnvPair(cmd, "LC_ALL", "C");
43fe83
 
43fe83
-    virCommandAddEnvPass(cmd, "LD_PRELOAD");
43fe83
-    virCommandAddEnvPass(cmd, "LD_LIBRARY_PATH");
43fe83
-    virCommandAddEnvPass(cmd, "PATH");
43fe83
-    virCommandAddEnvPass(cmd, "HOME");
43fe83
-    virCommandAddEnvPass(cmd, "USER");
43fe83
-    virCommandAddEnvPass(cmd, "LOGNAME");
43fe83
-    virCommandAddEnvPass(cmd, "TMPDIR");
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "LD_PRELOAD", NULL);
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "LD_LIBRARY_PATH", NULL);
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "PATH", "/bin:/usr/bin");
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "HOME", NULL);
43fe83
+    virCommandAddEnvPassAllowSUID(cmd, "USER");
43fe83
+    virCommandAddEnvPassAllowSUID(cmd, "LOGNAME");
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "TMPDIR", NULL);
43fe83
 }
43fe83
 
43fe83
 /**
43fe83
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
43fe83
index c619e06..e977f93 100644
43fe83
--- a/src/util/vircommand.h
43fe83
+++ b/src/util/vircommand.h
43fe83
@@ -99,8 +99,12 @@ void virCommandAddEnvString(virCommandPtr cmd,
43fe83
 void virCommandAddEnvBuffer(virCommandPtr cmd,
43fe83
                             virBufferPtr buf);
43fe83
 
43fe83
-void virCommandAddEnvPass(virCommandPtr cmd,
43fe83
-                          const char *name) ATTRIBUTE_NONNULL(2);
43fe83
+void virCommandAddEnvPassBlockSUID(virCommandPtr cmd,
43fe83
+                                   const char *name,
43fe83
+                                   const char *defvalue) ATTRIBUTE_NONNULL(2);
43fe83
+
43fe83
+void virCommandAddEnvPassAllowSUID(virCommandPtr cmd,
43fe83
+                                   const char *name) ATTRIBUTE_NONNULL(2);
43fe83
 
43fe83
 void virCommandAddEnvPassCommon(virCommandPtr cmd);
43fe83
 
43fe83
diff --git a/tests/commandtest.c b/tests/commandtest.c
43fe83
index eeb6d1e..1acc8d9 100644
43fe83
--- a/tests/commandtest.c
43fe83
+++ b/tests/commandtest.c
43fe83
@@ -294,8 +294,8 @@ static int test6(const void *unused ATTRIBUTE_UNUSED)
43fe83
 {
43fe83
     virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper");
43fe83
 
43fe83
-    virCommandAddEnvPass(cmd, "DISPLAY");
43fe83
-    virCommandAddEnvPass(cmd, "DOESNOTEXIST");
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL);
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "DOESNOTEXIST", NULL);
43fe83
 
43fe83
     if (virCommandRun(cmd, NULL) < 0) {
43fe83
         virErrorPtr err = virGetLastError();
43fe83
@@ -319,8 +319,8 @@ static int test7(const void *unused ATTRIBUTE_UNUSED)
43fe83
     virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper");
43fe83
 
43fe83
     virCommandAddEnvPassCommon(cmd);
43fe83
-    virCommandAddEnvPass(cmd, "DISPLAY");
43fe83
-    virCommandAddEnvPass(cmd, "DOESNOTEXIST");
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL);
43fe83
+    virCommandAddEnvPassBlockSUID(cmd, "DOESNOTEXIST", NULL);
43fe83
 
43fe83
     if (virCommandRun(cmd, NULL) < 0) {
43fe83
         virErrorPtr err = virGetLastError();
43fe83
-- 
43fe83
1.8.4.2
43fe83