[libvirt] [PATCH 07/17] util: simplify virCommand APIs for env passthrough.

Daniel P. Berrangé berrange at redhat.com
Thu Aug 1 15:00:09 UTC 2019


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.

Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
---
 src/libvirt_private.syms |  3 +--
 src/lxc/lxc_process.c    |  2 +-
 src/qemu/qemu_command.c  |  8 +++----
 src/rpc/virnetsocket.c   | 16 +++++++-------
 src/util/vircommand.c    | 46 ++++++++--------------------------------
 src/util/vircommand.h    |  8 ++-----
 tests/commandtest.c      |  8 +++----
 7 files changed, 29 insertions(+), 62 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8f344a07ee..983fe93d99 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1709,8 +1709,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 714eef20c8..a1d028b2e6 100644
--- 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 fee51158a9..f4b7e25cdf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8074,8 +8074,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");
@@ -8230,7 +8230,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");
 
@@ -10685,7 +10685,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 3282bc0817..ebd304707a 100644
--- 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 2df71014f8..ea9a9fd622 100644
--- 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 74574e3fb1..1a7158d4c1 100644
--- 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 d7ab588969..a382bb6dd2 100644
--- 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());
-- 
2.21.0




More information about the libvir-list mailing list