util: get rid of virIsSUID method

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.

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

diff --git a/src/libvirt.c b/src/libvirt.c
index 489785cec46bcb31f31ef88f7ae2ca9f58f7937d..161001bf48bdad0f9efc29143e470df7b122e162 100644 (file)
--- 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 156add9f359086aec566f9ced5e23c268ae9b6ed..ef053daa5017e0fbcbdf492bf898d9c3cbe0ae13 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3298,7 +3298,6 @@ virHostGetDRMRenderNode;
 virHostHasIOMMU;
 virIndexToDiskName;
 virIsDevMapperDevice;
-virIsSUID;
 virMemoryLimitIsSet;
 virMemoryLimitTruncate;
 virMemoryMaxValue;

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 72c2336b7a92591d80e79ff7cb606f5e46a7343a..5e6007d468666c61a45d83f1defcd48ba73c55eb 100644 (file)
--- 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 da433878df41c57061805b275c7bafecc63df6f6..6a2229ae2bfc9c1102cd7130e34f5712b7269bc7 100644 (file)
--- 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 84ccc1a5469af26495451cc5c7d58efb575613bc..4e0dbe15c4dc1b2d79250849a9bfb74a44049a63 100644 (file)
--- 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 7ea702f27a9909e6870c1bb8fbcec2be8a23162b..52d0c33773661504ea58e0d181c8108d7a9f3260 100644 (file)
--- 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);