util: simplify virCommand APIs for env passthrough.

Now that 100% of libvirt code is forbidden in a SUID environment,
we no longer need to worry about whether env variables are
trustworthy or not. The virt-login-shell setuid program, which
does not link to any libvirt code, will purge all environment
variables, except $TERM, before invoking the virt-login-shell-helper
program which uses libvirt.

Thus we only need one API for env passthrough in virCommand.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ef053daa5017e0fbcbdf492bf898d9c3cbe0ae13..3629751e708a14ef08dd2ffc19f55cde340ae5da 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1711,8 +1711,7 @@ virCommandAddArgSet;
 virCommandAddEnvBuffer;
 virCommandAddEnvFormat;
 virCommandAddEnvPair;
-virCommandAddEnvPassAllowSUID;
-virCommandAddEnvPassBlockSUID;
+virCommandAddEnvPass;
 virCommandAddEnvPassCommon;
 virCommandAddEnvString;
 virCommandAddEnvXDG;

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 714eef20c8b9ac16ea8d9c61e4ab30d279602781..a1d028b2e638a0a72fdc4099d5db59b1932c8329 100644 (file)
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -936,7 +936,7 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
     cmd = virCommandNew(vm->def->emulator);
 
     /* The controller may call ip command, so we have to retain PATH. */
-    virCommandAddEnvPassBlockSUID(cmd, "PATH", "/bin:/usr/bin");
+    virCommandAddEnvPass(cmd, "PATH");
 
     virCommandAddEnvFormat(cmd, "LIBVIRT_DEBUG=%d",
                            virLogGetDefaultPriority());

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c8494de785043fd149cc47dd45e51d3e31b85138..71a36ff63a819fe2270a5aecbb9da995443ddab2 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8075,8 +8075,8 @@ qemuBuildGraphicsSDLCommandLine(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED,
      * use QEMU's host audio drivers, possibly SDL too
      * User can set these two before starting libvirtd
      */
-    virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
-    virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL);
+    virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
+    virCommandAddEnvPass(cmd, "SDL_AUDIODRIVER");
 
     virCommandAddArg(cmd, "-display");
     virBufferAddLit(&opt, "sdl");
@@ -8231,7 +8231,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
      * security issues and might not work when using VNC.
      */
     if (cfg->vncAllowHostAudio)
-        virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
+        virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
     else
         virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
 
@@ -10686,7 +10686,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
         virCommandAddArg(cmd, "none");
 
         if (cfg->nogfxAllowHostAudio)
-            virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
+            virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
         else
             virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
     }

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 3282bc0817ba0bdcdd2575a4cc7d99397ef42e3c..ebd304707a0064b4759ab066a992bd74a33aac62 100644 (file)
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -141,9 +141,9 @@ static int virNetSocketForkDaemon(const char *binary)
                                              NULL);
 
     virCommandAddEnvPassCommon(cmd);
-    virCommandAddEnvPassBlockSUID(cmd, "XDG_CACHE_HOME", NULL);
-    virCommandAddEnvPassBlockSUID(cmd, "XDG_CONFIG_HOME", NULL);
-    virCommandAddEnvPassBlockSUID(cmd, "XDG_RUNTIME_DIR", NULL);
+    virCommandAddEnvPass(cmd, "XDG_CACHE_HOME");
+    virCommandAddEnvPass(cmd, "XDG_CONFIG_HOME");
+    virCommandAddEnvPass(cmd, "XDG_RUNTIME_DIR");
     virCommandClearCaps(cmd);
     virCommandDaemonize(cmd);
     ret = virCommandRun(cmd, NULL);
@@ -873,11 +873,11 @@ int virNetSocketNewConnectSSH(const char *nodename,
 
     cmd = virCommandNew(binary ? binary : "ssh");
     virCommandAddEnvPassCommon(cmd);
-    virCommandAddEnvPassBlockSUID(cmd, "KRB5CCNAME", NULL);
-    virCommandAddEnvPassBlockSUID(cmd, "SSH_AUTH_SOCK", NULL);
-    virCommandAddEnvPassBlockSUID(cmd, "SSH_ASKPASS", NULL);
-    virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL);
-    virCommandAddEnvPassBlockSUID(cmd, "XAUTHORITY", NULL);
+    virCommandAddEnvPass(cmd, "KRB5CCNAME");
+    virCommandAddEnvPass(cmd, "SSH_AUTH_SOCK");
+    virCommandAddEnvPass(cmd, "SSH_ASKPASS");
+    virCommandAddEnvPass(cmd, "DISPLAY");
+    virCommandAddEnvPass(cmd, "XAUTHORITY");
     virCommandClearCaps(cmd);
 
     if (service)

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 2df71014f84620d97fab39f5818ad11ea4796f62..ea9a9fd622fa6f17d04930ebf1727f48f94bab54 100644 (file)
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -1410,17 +1410,15 @@ virCommandAddEnvBuffer(virCommandPtr cmd, virBufferPtr buf)
 
 
 /**
- * virCommandAddEnvPassAllowSUID:
+ * virCommandAddEnvPass:
  * @cmd: the command to modify
  * @name: the name to look up in current environment
  *
  * Pass an environment variable to the child
  * using current process' value
- *
- * Allow to be passed even if setuid
  */
 void
-virCommandAddEnvPassAllowSUID(virCommandPtr cmd, const char *name)
+virCommandAddEnvPass(virCommandPtr cmd, const char *name)
 {
     const char *value;
     if (!cmd || cmd->has_error)
@@ -1432,32 +1430,6 @@ virCommandAddEnvPassAllowSUID(virCommandPtr cmd, const char *name)
 }
 
 
-/**
- * virCommandAddEnvPassBlockSUID:
- * @cmd: the command to modify
- * @name: the name to look up in current environment
- * @defvalue: value to return if running setuid, may be NULL
- *
- * Pass an environment variable to the child
- * using current process' value.
- *
- * Do not pass if running setuid
- */
-void
-virCommandAddEnvPassBlockSUID(virCommandPtr cmd, const char *name, const char *defvalue)
-{
-    const char *value;
-    if (!cmd || cmd->has_error)
-        return;
-
-    value = virGetEnvBlockSUID(name);
-    if (!value)
-        value = defvalue;
-    if (value)
-        virCommandAddEnvPair(cmd, name, value);
-}
-
-
 /**
  * virCommandAddEnvPassCommon:
  * @cmd: the command to modify
@@ -1478,13 +1450,13 @@ virCommandAddEnvPassCommon(virCommandPtr cmd)
 
     virCommandAddEnvPair(cmd, "LC_ALL", "C");
 
-    virCommandAddEnvPassBlockSUID(cmd, "LD_PRELOAD", NULL);
-    virCommandAddEnvPassBlockSUID(cmd, "LD_LIBRARY_PATH", NULL);
-    virCommandAddEnvPassBlockSUID(cmd, "PATH", "/bin:/usr/bin");
-    virCommandAddEnvPassBlockSUID(cmd, "HOME", NULL);
-    virCommandAddEnvPassAllowSUID(cmd, "USER");
-    virCommandAddEnvPassAllowSUID(cmd, "LOGNAME");
-    virCommandAddEnvPassBlockSUID(cmd, "TMPDIR", NULL);
+    virCommandAddEnvPass(cmd, "LD_PRELOAD");
+    virCommandAddEnvPass(cmd, "LD_LIBRARY_PATH");
+    virCommandAddEnvPass(cmd, "PATH");
+    virCommandAddEnvPass(cmd, "HOME");
+    virCommandAddEnvPass(cmd, "USER");
+    virCommandAddEnvPass(cmd, "LOGNAME");
+    virCommandAddEnvPass(cmd, "TMPDIR");
 }
 
 

diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index 74574e3fb135a313d0b9223630c96e0f750ef67c..1a7158d4c1bdc297115aec7ac55e089ce256ef43 100644 (file)
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -110,12 +110,8 @@ void virCommandAddEnvString(virCommandPtr cmd,
 void virCommandAddEnvBuffer(virCommandPtr cmd,
                             virBufferPtr buf);
 
-void virCommandAddEnvPassBlockSUID(virCommandPtr cmd,
-                                   const char *name,
-                                   const char *defvalue) ATTRIBUTE_NONNULL(2);
-
-void virCommandAddEnvPassAllowSUID(virCommandPtr cmd,
-                                   const char *name) ATTRIBUTE_NONNULL(2);
+void virCommandAddEnvPass(virCommandPtr cmd,
+                          const char *name) ATTRIBUTE_NONNULL(2);
 
 void virCommandAddEnvPassCommon(virCommandPtr cmd);
 

diff --git a/tests/commandtest.c b/tests/commandtest.c
index d7ab58896996c3e9cc3e32e6d01c4a15e46f33b8..a382bb6dd2ea2f27fc97453a7dabd96142ad7873 100644 (file)
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -305,8 +305,8 @@ static int test6(const void *unused ATTRIBUTE_UNUSED)
 {
     virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper");
 
-    virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL);
-    virCommandAddEnvPassBlockSUID(cmd, "DOESNOTEXIST", NULL);
+    virCommandAddEnvPass(cmd, "DISPLAY");
+    virCommandAddEnvPass(cmd, "DOESNOTEXIST");
 
     if (virCommandRun(cmd, NULL) < 0) {
         printf("Cannot run child %s\n", virGetLastErrorMessage());
@@ -329,8 +329,8 @@ static int test7(const void *unused ATTRIBUTE_UNUSED)
     virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper");
 
     virCommandAddEnvPassCommon(cmd);
-    virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL);
-    virCommandAddEnvPassBlockSUID(cmd, "DOESNOTEXIST", NULL);
+    virCommandAddEnvPass(cmd, "DISPLAY");
+    virCommandAddEnvPass(cmd, "DOESNOTEXIST");
 
     if (virCommandRun(cmd, NULL) < 0) {
         printf("Cannot run child %s\n", virGetLastErrorMessage());