Make virCommand env handling robust in setuid env

When running setuid, we must be careful about what env vars
we allow commands to inherit from us. Replace the
virCommandAddEnvPass function with two new ones which do
filtering

  virCommandAddEnvPassAllowSUID
  virCommandAddEnvPassBlockSUID

And make virCommandAddEnvPassCommon use the appropriate
ones

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrange 2013-10-09 11:03:02 +01:00
parent d665003da1
commit 9b8f307c6a
7 changed files with 64 additions and 31 deletions

View File

@ -1048,7 +1048,8 @@ virCommandAddArgSet;
virCommandAddEnvBuffer;
virCommandAddEnvFormat;
virCommandAddEnvPair;
virCommandAddEnvPass;
virCommandAddEnvPassAllowSUID;
virCommandAddEnvPassBlockSUID;
virCommandAddEnvPassCommon;
virCommandAddEnvString;
virCommandAllowCap;

View File

@ -740,7 +740,7 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
cmd = virCommandNew(vm->def->emulator);
/* The controller may call ip command, so we have to retain PATH. */
virCommandAddEnvPass(cmd, "PATH");
virCommandAddEnvPassBlockSUID(cmd, "PATH", "/bin:/usr/bin");
virCommandAddEnvFormat(cmd, "LIBVIRT_DEBUG=%d",
virLogGetDefaultPriority());

View File

@ -7141,7 +7141,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
* security issues and might not work when using VNC.
*/
if (cfg->vncAllowHostAudio)
virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
else
virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
@ -7396,8 +7396,8 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
* use QEMU's host audio drivers, possibly SDL too
* User can set these two before starting libvirtd
*/
virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
virCommandAddEnvPass(cmd, "SDL_AUDIODRIVER");
virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL);
/* New QEMU has this flag to let us explicitly ask for
* SDL graphics. This is better than relying on the
@ -7847,7 +7847,7 @@ qemuBuildCommandLine(virConnectPtr conn,
virCommandAddArg(cmd, "-nographic");
if (cfg->nogfxAllowHostAudio)
virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL);
else
virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
}

View File

@ -127,9 +127,9 @@ static int virNetSocketForkDaemon(const char *binary)
NULL);
virCommandAddEnvPassCommon(cmd);
virCommandAddEnvPass(cmd, "XDG_CACHE_HOME");
virCommandAddEnvPass(cmd, "XDG_CONFIG_HOME");
virCommandAddEnvPass(cmd, "XDG_RUNTIME_DIR");
virCommandAddEnvPassBlockSUID(cmd, "XDG_CACHE_HOME", NULL);
virCommandAddEnvPassBlockSUID(cmd, "XDG_CONFIG_HOME", NULL);
virCommandAddEnvPassBlockSUID(cmd, "XDG_RUNTIME_DIR", NULL);
virCommandClearCaps(cmd);
virCommandDaemonize(cmd);
ret = virCommandRun(cmd, NULL);
@ -680,11 +680,11 @@ int virNetSocketNewConnectSSH(const char *nodename,
cmd = virCommandNew(binary ? binary : "ssh");
virCommandAddEnvPassCommon(cmd);
virCommandAddEnvPass(cmd, "KRB5CCNAME");
virCommandAddEnvPass(cmd, "SSH_AUTH_SOCK");
virCommandAddEnvPass(cmd, "SSH_ASKPASS");
virCommandAddEnvPass(cmd, "DISPLAY");
virCommandAddEnvPass(cmd, "XAUTHORITY");
virCommandAddEnvPassBlockSUID(cmd, "KRB5CCNAME", NULL);
virCommandAddEnvPassBlockSUID(cmd, "SSH_AUTH_SOCK", NULL);
virCommandAddEnvPassBlockSUID(cmd, "SSH_ASKPASS", NULL);
virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL);
virCommandAddEnvPassBlockSUID(cmd, "XAUTHORITY", NULL);
virCommandClearCaps(cmd);
if (service)

View File

@ -1246,21 +1246,49 @@ virCommandAddEnvBuffer(virCommandPtr cmd, virBufferPtr buf)
/**
* virCommandAddEnvPass:
* virCommandAddEnvPassAllowSUID:
* @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
virCommandAddEnvPass(virCommandPtr cmd, const char *name)
virCommandAddEnvPassAllowSUID(virCommandPtr cmd, const char *name)
{
char *value;
const char *value;
if (!cmd || cmd->has_error)
return;
value = getenv(name);
value = virGetEnvAllowSUID(name);
if (value)
virCommandAddEnvPair(cmd, name, value);
}
/**
* 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);
}
@ -1286,13 +1314,13 @@ virCommandAddEnvPassCommon(virCommandPtr cmd)
virCommandAddEnvPair(cmd, "LC_ALL", "C");
virCommandAddEnvPass(cmd, "LD_PRELOAD");
virCommandAddEnvPass(cmd, "LD_LIBRARY_PATH");
virCommandAddEnvPass(cmd, "PATH");
virCommandAddEnvPass(cmd, "HOME");
virCommandAddEnvPass(cmd, "USER");
virCommandAddEnvPass(cmd, "LOGNAME");
virCommandAddEnvPass(cmd, "TMPDIR");
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);
}
/**

View File

@ -99,8 +99,12 @@ void virCommandAddEnvString(virCommandPtr cmd,
void virCommandAddEnvBuffer(virCommandPtr cmd,
virBufferPtr buf);
void virCommandAddEnvPass(virCommandPtr cmd,
const char *name) ATTRIBUTE_NONNULL(2);
void virCommandAddEnvPassBlockSUID(virCommandPtr cmd,
const char *name,
const char *defvalue) ATTRIBUTE_NONNULL(2);
void virCommandAddEnvPassAllowSUID(virCommandPtr cmd,
const char *name) ATTRIBUTE_NONNULL(2);
void virCommandAddEnvPassCommon(virCommandPtr cmd);

View File

@ -294,8 +294,8 @@ static int test6(const void *unused ATTRIBUTE_UNUSED)
{
virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper");
virCommandAddEnvPass(cmd, "DISPLAY");
virCommandAddEnvPass(cmd, "DOESNOTEXIST");
virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL);
virCommandAddEnvPassBlockSUID(cmd, "DOESNOTEXIST", NULL);
if (virCommandRun(cmd, NULL) < 0) {
virErrorPtr err = virGetLastError();
@ -319,8 +319,8 @@ static int test7(const void *unused ATTRIBUTE_UNUSED)
virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper");
virCommandAddEnvPassCommon(cmd);
virCommandAddEnvPass(cmd, "DISPLAY");
virCommandAddEnvPass(cmd, "DOESNOTEXIST");
virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL);
virCommandAddEnvPassBlockSUID(cmd, "DOESNOTEXIST", NULL);
if (virCommandRun(cmd, NULL) < 0) {
virErrorPtr err = virGetLastError();