util: get rid of virGetEnv{Allow,Block}SUID functions

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.

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

cfg.mk

@@ -855,12 +855,6 @@ sc_prohibit_unbounded_arrays_in_rpc:
 	halt='Arrays in XDR must have a upper limit set for <NNN>' \
 	  $(_sc_search_regexp)
 
-sc_prohibit_getenv:
-	@prohibit='\b(secure_)?getenv *\(' \
-	exclude='exempt from syntax-check' \
-	halt='Use virGetEnv{Allow,Block}SUID instead of getenv' \
-	  $(_sc_search_regexp)
-
 sc_prohibit_atoi:
 	@prohibit='\bato(i|f|l|ll|q) *\(' \
 	halt='Use virStrToLong* instead of atoi, atol, atof, atoq, atoll' \
@@ -1316,9 +1310,6 @@ exclude_file_name_regexp--sc_prohibit_int_ijk = \
 exclude_file_name_regexp--sc_prohibit_unsigned_pid = \
   ^(include/libvirt/.*\.h|src/(qemu/qemu_driver\.c|driver-hypervisor\.h|libvirt(-[a-z]*)?\.c|.*\.x|util/vir(polkit|systemd)\.c)|tests/virpolkittest\.c|tools/virsh-domain\.c)$$
 
-exclude_file_name_regexp--sc_prohibit_getenv = \
-  ^tests/.*\.[ch]|tools/virt-login-shell\.c$$
-
 exclude_file_name_regexp--sc_avoid_attribute_unused_in_header = \
   ^(src/util/virlog\.h|src/network/bridge_driver\.h)$$
 

src/libvirt-admin.c

@@ -169,7 +169,7 @@ getSocketPath(virURIPtr uri)
 static int
 virAdmGetDefaultURI(virConfPtr conf, char **uristr)
 {
-    const char *defname = virGetEnvAllowSUID("LIBVIRT_ADMIN_DEFAULT_URI");
+    const char *defname = getenv("LIBVIRT_ADMIN_DEFAULT_URI");
     if (defname && *defname) {
         if (VIR_STRDUP(*uristr, defname) < 0)
             return -1;

src/libvirt.c

@@ -774,7 +774,7 @@ virConnectGetDefaultURI(virConfPtr conf,
                         char **name)
 {
     int ret = -1;
-    const char *defname = virGetEnvBlockSUID("LIBVIRT_DEFAULT_URI");
+    const char *defname = getenv("LIBVIRT_DEFAULT_URI");
     if (defname && *defname) {
         VIR_DEBUG("Using LIBVIRT_DEFAULT_URI '%s'", defname);
         if (VIR_STRDUP(*name, defname) < 0)

src/libvirt_private.syms

@@ -3273,8 +3273,6 @@ virFormatIntDecimal;
 virFormatIntPretty;
 virGetDeviceID;
 virGetDeviceUnprivSGIO;
-virGetEnvAllowSUID;
-virGetEnvBlockSUID;
 virGetGroupID;
 virGetGroupList;
 virGetGroupName;

src/network/leaseshelper.c

@@ -86,10 +86,10 @@ main(int argc, char **argv)
     const char *ip = NULL;
     const char *mac = NULL;
     const char *leases_str = NULL;
-    const char *iaid = virGetEnvAllowSUID("DNSMASQ_IAID");
+    const char *iaid = getenv("DNSMASQ_IAID");
-    const char *clientid = virGetEnvAllowSUID("DNSMASQ_CLIENT_ID");
+    const char *clientid = getenv("DNSMASQ_CLIENT_ID");
-    const char *interface = virGetEnvAllowSUID("DNSMASQ_INTERFACE");
+    const char *interface = getenv("DNSMASQ_INTERFACE");
-    const char *hostname = virGetEnvAllowSUID("DNSMASQ_SUPPLIED_HOSTNAME");
+    const char *hostname = getenv("DNSMASQ_SUPPLIED_HOSTNAME");
     char *server_duid = NULL;
     int action = -1;
     int pid_file_fd = -1;
@@ -131,7 +131,7 @@ main(int argc, char **argv)
      * events for expired leases. So, libvirtd sets another env var for this
      * purpose */
     if (!interface &&
-        !(interface = virGetEnvAllowSUID("VIR_BRIDGE_NAME")))
+        !(interface = getenv("VIR_BRIDGE_NAME")))
         goto cleanup;
 
     ip = argv[3];
@@ -148,11 +148,11 @@ main(int argc, char **argv)
 
     /* Check if it is an IPv6 lease */
     if (iaid) {
-        mac = virGetEnvAllowSUID("DNSMASQ_MAC");
+        mac = getenv("DNSMASQ_MAC");
         clientid = argv[2];
     }
 
-    if (VIR_STRDUP(server_duid, virGetEnvAllowSUID("DNSMASQ_SERVER_DUID")) < 0)
+    if (VIR_STRDUP(server_duid, getenv("DNSMASQ_SERVER_DUID")) < 0)
         goto cleanup;
 
     if (virAsprintf(&custom_lease_file,

src/qemu/qemu_firmware.c

@@ -986,7 +986,7 @@ qemuFirmwareFetchConfigs(char ***firmwares,
          * much sense to parse files in root's home directory. It
          * makes sense only for session daemon which runs under
          * regular user. */
-        if (VIR_STRDUP(xdgConfig, virGetEnvBlockSUID("XDG_CONFIG_HOME")) < 0)
+        if (VIR_STRDUP(xdgConfig, getenv("XDG_CONFIG_HOME")) < 0)
             return -1;
 
         if (!xdgConfig) {

src/remote/remote_driver.c

@@ -1297,7 +1297,7 @@ remoteConnectOpen(virConnectPtr conn,
     struct private_data *priv;
     int ret = VIR_DRV_OPEN_ERROR;
     int rflags = 0;
-    const char *autostart = virGetEnvBlockSUID("LIBVIRT_AUTOSTART");
+    const char *autostart = getenv("LIBVIRT_AUTOSTART");
     char *driver = NULL;
     char *transport = NULL;
 

src/rpc/virnetlibsshsession.c

@@ -172,7 +172,7 @@ virNetLibsshSessionOnceInit(void)
     ssh_set_log_level(TRACE_LIBSSH);
 #endif
 
-    dbgLevelStr = virGetEnvAllowSUID("LIBVIRT_LIBSSH_DEBUG");
+    dbgLevelStr = getenv("LIBVIRT_LIBSSH_DEBUG");
     if (dbgLevelStr &&
         virStrToLong_i(dbgLevelStr, NULL, 10, &dbgLevel) >= 0)
         ssh_set_log_level(dbgLevel);

src/rpc/virnettlscontext.c

@@ -1439,7 +1439,7 @@ void virNetTLSSessionDispose(void *obj)
 void virNetTLSInit(void)
 {
     const char *gnutlsdebug;
-    if ((gnutlsdebug = virGetEnvAllowSUID("LIBVIRT_GNUTLS_DEBUG")) != NULL) {
+    if ((gnutlsdebug = getenv("LIBVIRT_GNUTLS_DEBUG")) != NULL) {
         int val;
         if (virStrToLong_i(gnutlsdebug, NULL, 10, &val) < 0)
             val = 10;

src/util/virauth.c

@@ -42,7 +42,7 @@ virAuthGetConfigFilePathURI(virURIPtr uri,
                             char **path)
 {
     size_t i;
-    const char *authenv = virGetEnvBlockSUID("LIBVIRT_AUTH_FILE");
+    const char *authenv = getenv("LIBVIRT_AUTH_FILE");
     VIR_AUTOFREE(char *) userdir = NULL;
 
     *path = NULL;

src/util/vircommand.c

@@ -1424,7 +1424,7 @@ virCommandAddEnvPass(virCommandPtr cmd, const char *name)
     if (!cmd || cmd->has_error)
         return;
 
-    value = virGetEnvAllowSUID(name);
+    value = getenv(name);
     if (value)
         virCommandAddEnvPair(cmd, name, value);
 }

src/util/virfile.c

@@ -1674,7 +1674,7 @@ virFindFileInPath(const char *file)
     }
 
     /* copy PATH env so we can tweak it */
-    origpath = virGetEnvBlockSUID("PATH");
+    origpath = getenv("PATH");
     if (!origpath)
         origpath = "/bin:/usr/bin";
 
@@ -1735,7 +1735,7 @@ virFileFindResourceFull(const char *filename,
                         const char *envname)
 {
     char *ret = NULL;
-    const char *envval = envname ? virGetEnvBlockSUID(envname) : NULL;
+    const char *envval = envname ? getenv(envname) : NULL;
     const char *path;
 
     if (!prefix)

src/util/virlease.c

@@ -213,13 +213,13 @@ virLeaseNew(virJSONValuePtr *lease_ret,
             const char *server_duid)
 {
     VIR_AUTOPTR(virJSONValue) lease_new = NULL;
-    const char *exptime_tmp = virGetEnvAllowSUID("DNSMASQ_LEASE_EXPIRES");
+    const char *exptime_tmp = getenv("DNSMASQ_LEASE_EXPIRES");
     long long expirytime = 0;
     VIR_AUTOFREE(char *) exptime = NULL;
 
     /* In case hostname is still unknown, use the last known one */
     if (!hostname)
-        hostname = virGetEnvAllowSUID("DNSMASQ_OLD_HOSTNAME");
+        hostname = getenv("DNSMASQ_OLD_HOSTNAME");
 
     if (!mac)
         return 0;

src/util/virlog.c

@@ -1308,13 +1308,13 @@ virLogSetFromEnv(void)
     if (virLogInitialize() < 0)
         return;
 
-    debugEnv = virGetEnvAllowSUID("LIBVIRT_DEBUG");
+    debugEnv = getenv("LIBVIRT_DEBUG");
     if (debugEnv && *debugEnv)
         virLogSetDefaultPriority(virLogParseDefaultPriority(debugEnv));
-    debugEnv = virGetEnvAllowSUID("LIBVIRT_LOG_FILTERS");
+    debugEnv = getenv("LIBVIRT_LOG_FILTERS");
     if (debugEnv && *debugEnv)
         virLogSetFilters(debugEnv);
-    debugEnv = virGetEnvAllowSUID("LIBVIRT_LOG_OUTPUTS");
+    debugEnv = getenv("LIBVIRT_LOG_OUTPUTS");
     if (debugEnv && *debugEnv)
         virLogSetOutputs(debugEnv);
 }

src/util/virsystemd.c

@@ -509,7 +509,7 @@ virSystemdNotifyStartup(void)
         .msg_iovlen = 1,
     };
 
-    if (!(path = virGetEnvBlockSUID("NOTIFY_SOCKET"))) {
+    if (!(path = getenv("NOTIFY_SOCKET"))) {
         VIR_DEBUG("Skipping systemd notify, not requested");
         return;
     }
@@ -798,7 +798,7 @@ virSystemdGetListenFDs(void)
 
     VIR_DEBUG("Setting up networking from caller");
 
-    if (!(pidstr = virGetEnvAllowSUID("LISTEN_PID"))) {
+    if (!(pidstr = getenv("LISTEN_PID"))) {
         VIR_DEBUG("No LISTEN_PID from caller");
         return 0;
     }
@@ -814,7 +814,7 @@ virSystemdGetListenFDs(void)
         return 0;
     }
 
-    if (!(fdstr = virGetEnvAllowSUID("LISTEN_FDS"))) {
+    if (!(fdstr = getenv("LISTEN_FDS"))) {
         VIR_DEBUG("No LISTEN_FDS from caller");
         return 0;
     }
@@ -866,7 +866,7 @@ virSystemdActivationNew(virSystemdActivationMap *map,
     if (!(act->fds = virHashCreate(10, virSystemdActivationEntryFree)))
         goto error;
 
-    fdnames = virGetEnvAllowSUID("LISTEN_FDNAMES");
+    fdnames = getenv("LISTEN_FDNAMES");
     if (fdnames) {
         if (virSystemdActivationInitFromNames(act, nfds, fdnames) < 0)
             goto error;

src/util/virutil.c

@@ -739,7 +739,7 @@ char *virGetUserShell(uid_t uid)
 
 static char *virGetXDGDirectory(const char *xdgenvname, const char *xdgdefdir)
 {
-    const char *path = virGetEnvBlockSUID(xdgenvname);
+    const char *path = getenv(xdgenvname);
     char *ret = NULL;
     char *home = NULL;
 
@@ -767,7 +767,7 @@ char *virGetUserCacheDirectory(void)
 
 char *virGetUserRuntimeDirectory(void)
 {
-    const char *path = virGetEnvBlockSUID("XDG_RUNTIME_DIR");
+    const char *path = getenv("XDG_RUNTIME_DIR");
 
     if (!path || !path[0]) {
         return virGetUserCacheDirectory();
@@ -1137,7 +1137,7 @@ virGetUserDirectoryByUID(uid_t uid ATTRIBUTE_UNUSED)
     const char *dir;
     char *ret;
 
-    dir = virGetEnvBlockSUID("HOME");
+    dir = getenv("HOME");
 
     /* Only believe HOME if it is an absolute path and exists */
     if (dir) {
@@ -1157,7 +1157,7 @@ virGetUserDirectoryByUID(uid_t uid ATTRIBUTE_UNUSED)
 
     if (!dir)
         /* USERPROFILE is probably the closest equivalent to $HOME? */
-        dir = virGetEnvBlockSUID("USERPROFILE");
+        dir = getenv("USERPROFILE");
 
     if (VIR_STRDUP(ret, dir) < 0)
         return NULL;
@@ -1722,34 +1722,6 @@ virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr)
     return rc;
 }
 
-
-/**
- * virGetEnvBlockSUID:
- * @name: the environment variable name
- *
- * Obtain an environment variable which is unsafe to
- * use when running setuid. If running setuid, a NULL
- * value will be returned
- */
-const char *virGetEnvBlockSUID(const char *name)
-{
-    return secure_getenv(name); /* exempt from syntax-check */
-}
-
-
-/**
- * virGetEnvAllowSUID:
- * @name: the environment variable name
- *
- * Obtain an environment variable which is safe to
- * use when running setuid. The value will be returned
- * even when running setuid
- */
-const char *virGetEnvAllowSUID(const char *name)
-{
-    return getenv(name); /* exempt from syntax-check */
-}
-
 static time_t selfLastChanged;
 
 time_t virGetSelfLastChanged(void)

src/util/virutil.h

@@ -141,9 +141,6 @@ char *virGetUnprivSGIOSysfsPath(const char *path,
 
 int virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr);
 
-const char *virGetEnvBlockSUID(const char *name);
-const char *virGetEnvAllowSUID(const char *name);
-
 
 time_t virGetSelfLastChanged(void);
 void virUpdateSelfLastChanged(const char *path);

src/vbox/vbox_XPCOMCGlue.c

@@ -190,7 +190,7 @@ VBoxCGlueInit(unsigned int *version)
         "/usr/local/lib/VirtualBox",
         "/Applications/VirtualBox.app/Contents/MacOS"
     };
-    const char *home = virGetEnvBlockSUID("VBOX_APP_HOME");
+    const char *home = getenv("VBOX_APP_HOME");
 
     /* If the user specifies the location, try only that. */
     if (home != NULL) {

src/vbox/vbox_common.c

@@ -3558,7 +3558,7 @@ vboxDumpDisplay(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
 
         graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
         if (VIR_STRDUP(graphics->data.desktop.display,
-                       virGetEnvBlockSUID("DISPLAY")) < 0)
+                       getenv("DISPLAY")) < 0)
             goto cleanup;
     }
 

tools/virsh.c

@@ -913,7 +913,7 @@ main(int argc, char **argv)
 
     if (!ctl->connname)
         ctl->connname = vshStrdup(ctl,
-                                  virGetEnvBlockSUID("VIRSH_DEFAULT_CONNECT_URI"));
+                                  getenv("VIRSH_DEFAULT_CONNECT_URI"));
 
     if (!ctl->imode) {
         ret = vshCommandRun(ctl, ctl->cmd);

tools/virt-login-shell-helper.c

@@ -372,8 +372,8 @@ main(int argc, char **argv)
 
     /* We're duping the string because the clearenv()
      * call will shortly release the pointer we get
-     * back from virGetEnvAllowSUID() right here */
+     * back from getenv() right here */
-    if (VIR_STRDUP(term, virGetEnvAllowSUID("TERM")) < 0)
+    if (VIR_STRDUP(term, getenv("TERM")) < 0)
         goto cleanup;
 
     /* A fork is required to create new process in correct pid namespace.  */

tools/vsh.c

@@ -2429,7 +2429,7 @@ vshEditWriteToTempFile(vshControl *ctl, const char *doc)
     int fd;
     char ebuf[1024];
 
-    tmpdir = virGetEnvBlockSUID("TMPDIR");
+    tmpdir = getenv("TMPDIR");
     if (!tmpdir) tmpdir = "/tmp";
     if (virAsprintf(&ret, "%s/virshXXXXXX.xml", tmpdir) < 0) {
         vshError(ctl, "%s", _("out of memory"));
@@ -2476,9 +2476,9 @@ vshEditFile(vshControl *ctl, const char *filename)
     int outfd = STDOUT_FILENO;
     int errfd = STDERR_FILENO;
 
-    editor = virGetEnvBlockSUID("VISUAL");
+    editor = getenv("VISUAL");
     if (!editor)
-        editor = virGetEnvBlockSUID("EDITOR");
+        editor = getenv("EDITOR");
     if (!editor)
         editor = DEFAULT_EDITOR;
 
@@ -2956,7 +2956,7 @@ vshReadlineInit(vshControl *ctl)
         goto cleanup;
 
     /* Limit the total size of the history buffer */
-    if ((histsize_str = virGetEnvBlockSUID(histsize_env))) {
+    if ((histsize_str = getenv(histsize_env))) {
         if (virStrToLong_i(histsize_str, NULL, 10, &max_history) < 0) {
             vshError(ctl, _("Bad $%s value."), histsize_env);
             goto cleanup;
@@ -3072,7 +3072,7 @@ vshInitDebug(vshControl *ctl)
             return -1;
 
         /* log level not set from commandline, check env variable */
-        debugEnv = virGetEnvAllowSUID(env);
+        debugEnv = getenv(env);
         if (debugEnv) {
             int debug;
             if (virStrToLong_i(debugEnv, NULL, 10, &debug) < 0 ||
@@ -3091,7 +3091,7 @@ vshInitDebug(vshControl *ctl)
             return -1;
 
         /* log file not set from cmdline */
-        debugEnv = virGetEnvBlockSUID(env);
+        debugEnv = getenv(env);
         if (debugEnv && *debugEnv) {
             ctl->logfile = vshStrdup(ctl, debugEnv);
             vshOpenLogFile(ctl);