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>
This commit is contained in:
Daniel P. Berrangé 2019-08-01 13:52:00 +01:00
parent 0c69168486
commit fcf93c3ee0
7 changed files with 29 additions and 62 deletions

View File

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

View File

@ -936,7 +936,7 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
cmd = virCommandNew(vm->def->emulator); cmd = virCommandNew(vm->def->emulator);
/* The controller may call ip command, so we have to retain PATH. */ /* 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", virCommandAddEnvFormat(cmd, "LIBVIRT_DEBUG=%d",
virLogGetDefaultPriority()); virLogGetDefaultPriority());

View File

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

View File

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

View File

@ -1410,17 +1410,15 @@ virCommandAddEnvBuffer(virCommandPtr cmd, virBufferPtr buf)
/** /**
* virCommandAddEnvPassAllowSUID: * virCommandAddEnvPass:
* @cmd: the command to modify * @cmd: the command to modify
* @name: the name to look up in current environment * @name: the name to look up in current environment
* *
* Pass an environment variable to the child * Pass an environment variable to the child
* using current process' value * using current process' value
*
* Allow to be passed even if setuid
*/ */
void void
virCommandAddEnvPassAllowSUID(virCommandPtr cmd, const char *name) virCommandAddEnvPass(virCommandPtr cmd, const char *name)
{ {
const char *value; const char *value;
if (!cmd || cmd->has_error) 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: * virCommandAddEnvPassCommon:
* @cmd: the command to modify * @cmd: the command to modify
@ -1478,13 +1450,13 @@ virCommandAddEnvPassCommon(virCommandPtr cmd)
virCommandAddEnvPair(cmd, "LC_ALL", "C"); virCommandAddEnvPair(cmd, "LC_ALL", "C");
virCommandAddEnvPassBlockSUID(cmd, "LD_PRELOAD", NULL); virCommandAddEnvPass(cmd, "LD_PRELOAD");
virCommandAddEnvPassBlockSUID(cmd, "LD_LIBRARY_PATH", NULL); virCommandAddEnvPass(cmd, "LD_LIBRARY_PATH");
virCommandAddEnvPassBlockSUID(cmd, "PATH", "/bin:/usr/bin"); virCommandAddEnvPass(cmd, "PATH");
virCommandAddEnvPassBlockSUID(cmd, "HOME", NULL); virCommandAddEnvPass(cmd, "HOME");
virCommandAddEnvPassAllowSUID(cmd, "USER"); virCommandAddEnvPass(cmd, "USER");
virCommandAddEnvPassAllowSUID(cmd, "LOGNAME"); virCommandAddEnvPass(cmd, "LOGNAME");
virCommandAddEnvPassBlockSUID(cmd, "TMPDIR", NULL); virCommandAddEnvPass(cmd, "TMPDIR");
} }

View File

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

View File

@ -305,8 +305,8 @@ static int test6(const void *unused ATTRIBUTE_UNUSED)
{ {
virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper");
virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL); virCommandAddEnvPass(cmd, "DISPLAY");
virCommandAddEnvPassBlockSUID(cmd, "DOESNOTEXIST", NULL); virCommandAddEnvPass(cmd, "DOESNOTEXIST");
if (virCommandRun(cmd, NULL) < 0) { if (virCommandRun(cmd, NULL) < 0) {
printf("Cannot run child %s\n", virGetLastErrorMessage()); 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"); virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper");
virCommandAddEnvPassCommon(cmd); virCommandAddEnvPassCommon(cmd);
virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL); virCommandAddEnvPass(cmd, "DISPLAY");
virCommandAddEnvPassBlockSUID(cmd, "DOESNOTEXIST", NULL); virCommandAddEnvPass(cmd, "DOESNOTEXIST");
if (virCommandRun(cmd, NULL) < 0) { if (virCommandRun(cmd, NULL) < 0) {
printf("Cannot run child %s\n", virGetLastErrorMessage()); printf("Cannot run child %s\n", virGetLastErrorMessage());