[PATCH 06/17] util: get rid of virIsSUID method

[Daniel P. Berrangé <berrange@xxxxxxxxxx>]

Now that none of the libvirt.so code will ever run in a setuid
context, we can remove the virIsSUID() method. The global
initializer function can just inline the check itself. The new
inlined check is slightly stronger as it also looks for a
setgid situation.

Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
---
 src/libvirt.c              | 13 +++----------
 src/libvirt_private.syms   |  1 -
 src/remote/remote_driver.c | 23 +++--------------------
 src/util/virlog.c          |  9 ---------
 src/util/virutil.c         | 12 ------------
 src/util/virutil.h         |  1 -
 6 files changed, 6 insertions(+), 53 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index 489785cec4..161001bf48 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -250,13 +250,12 @@ virGlobalInit(void)
         virErrorInitialize() < 0)
         goto error;
 
-#ifndef LIBVIRT_SETUID_RPC_CLIENT
-    if (virIsSUID()) {
+    if (getuid() != geteuid() ||
+        getgid() != getegid()) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("libvirt.so is not safe to use from setuid programs"));
+                       _("libvirt.so is not safe to use from setuid/setgid programs"));
         goto error;
     }
-#endif
 
     virLogSetFromEnv();
 
@@ -844,12 +843,6 @@ virConnectOpenInternal(const char *name,
     if (name && name[0] == '\0')
         name = NULL;
 
-    if (!name && virIsSUID()) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("An explicit URI must be provided when setuid"));
-        goto failed;
-    }
-
     /* Convert xen -> xen:///system for back compat */
     if (name && STRCASEEQ(name, "xen"))
         name = "xen:///system";
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c323f679b3..8f344a07ee 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3295,7 +3295,6 @@ virHostGetDRMRenderNode;
 virHostHasIOMMU;
 virIndexToDiskName;
 virIsDevMapperDevice;
-virIsSUID;
 virMemoryLimitIsSet;
 virMemoryLimitTruncate;
 virMemoryMaxValue;
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 72c2336b7a..5e6007d468 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -853,21 +853,6 @@ doRemoteOpen(virConnectPtr conn,
         transport = trans_unix;
     }
 
-    /*
-     * We don't want to be executing external programs in setuid mode,
-     * so this rules out 'ext' and 'ssh' transports. Exclude libssh
-     * and tls too, since we're not confident the libraries are safe
-     * for setuid usage. Just allow UNIX sockets, since that does
-     * not require any external libraries or command execution
-     */
-    if (virIsSUID() &&
-        transport != trans_unix) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Only Unix socket URI transport is allowed in setuid mode"));
-        return VIR_DRV_OPEN_ERROR;
-    }
-
-
     /* Remote server defaults to "localhost" if not specified. */
     if (conn->uri && conn->uri->port != 0) {
         if (virAsprintf(&port, "%d", conn->uri->port) < 0)
@@ -1353,8 +1338,7 @@ remoteConnectOpen(virConnectPtr conn,
          * transport is listed, or transport is unix,
          * and uid is unprivileged then auto-spawn a daemon.
          */
-        if (!virIsSUID() &&
-            !conn->uri->server &&
+        if (!conn->uri->server &&
             (transport == NULL || STREQ(transport, "unix")) &&
             (!autostart ||
              STRNEQ(autostart, "0"))) {
@@ -1372,9 +1356,8 @@ remoteConnectOpen(virConnectPtr conn,
         if (geteuid() > 0) {
             VIR_DEBUG("Auto-spawn user daemon instance");
             rflags |= VIR_DRV_OPEN_REMOTE_USER;
-            if (!virIsSUID() &&
-                (!autostart ||
-                 STRNEQ(autostart, "0")))
+            if (!autostart ||
+                STRNEQ(autostart, "0"))
                 rflags |= VIR_DRV_OPEN_REMOTE_AUTOSTART;
         }
     }
diff --git a/src/util/virlog.c b/src/util/virlog.c
index da433878df..6a2229ae2b 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1588,7 +1588,6 @@ virLogParseOutput(const char *src)
     size_t count = 0;
     virLogPriority prio;
     int dest;
-    bool isSUID = virIsSUID();
 
     VIR_DEBUG("output=%s", src);
 
@@ -1626,14 +1625,6 @@ virLogParseOutput(const char *src)
         goto cleanup;
     }
 
-    /* if running with setuid, only 'stderr' is allowed */
-    if (isSUID && dest != VIR_LOG_TO_STDERR) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Running with SUID permits only destination of type "
-                         "'stderr'"));
-        goto cleanup;
-    }
-
     switch ((virLogDestination) dest) {
     case VIR_LOG_TO_STDERR:
         ret = virLogNewOutputToStderr(prio);
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 84ccc1a546..4e0dbe15c4 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1750,18 +1750,6 @@ const char *virGetEnvAllowSUID(const char *name)
     return getenv(name); /* exempt from syntax-check */
 }
 
-
-/**
- * virIsSUID:
- * Return a true value if running setuid. Does not
- * check for elevated capabilities bits.
- */
-bool virIsSUID(void)
-{
-    return getuid() != geteuid();
-}
-
-
 static time_t selfLastChanged;
 
 time_t virGetSelfLastChanged(void)
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 7ea702f27a..52d0c33773 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -143,7 +143,6 @@ int virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr);
 
 const char *virGetEnvBlockSUID(const char *name);
 const char *virGetEnvAllowSUID(const char *name);
-bool virIsSUID(void);
 
 
 time_t virGetSelfLastChanged(void);
-- 
2.21.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list