[libvirt] [PATCH] remote: remove some __sun conditionals

Daniel P. Berrangé posted 1 patch 6 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180323110254.14589-1-berrange@redhat.com
Test syntax-check passed
src/remote/remote_daemon.c | 33 ---------------------------------
src/remote/remote_driver.c |  2 --
2 files changed, 35 deletions(-)
[libvirt] [PATCH] remote: remove some __sun conditionals
Posted by Daniel P. Berrangé 6 years ago
The libvirtd daemon has some arbitrary logic to drop privileges, but
only on Solaris platforms. This was added during Xen days, when Xen was
the only driver running in libvirtd. There's no expectation or testing
that this works with the new libxl stack, nor whether dropping
privileges breaks any of the secondary drivers. Finally, we'll be
splitting drivers out into their own independant daemons, so this won't
be applicable to libvirtd in future anyway.

The remote driver client meanwhile arbitrarily disables daemon
auto-spawn when connecting as non-root, breaking a key feature of
libvirt unprivileged connections.

Since we've not had any contributions for Solaris since circa 2012
and we don't do any CI testing we should consider this platform
unmaintained and thus reasonable to remove this cruft. If someone steps
forward to maintain Solaris again, this code would need re-evaluating to
come up with something more targetted.

There's various __sun conditionals in the Xen driver code, but those are
not touched. This is all for the legacy Xen driver, which will be
entirely removed at some point in future, so not benefit to hacking out
just the Solaris parts.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/remote/remote_daemon.c | 33 ---------------------------------
 src/remote/remote_driver.c |  2 --
 2 files changed, 35 deletions(-)

diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index 2f9dfad443..31c6ce1b61 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -675,30 +675,6 @@ daemonVersion(const char *argv0)
     printf("%s (%s) %s\n", argv0, PACKAGE_NAME, PACKAGE_VERSION);
 }
 
-#ifdef __sun
-static int
-daemonSetupPrivs(void)
-{
-    chown("/var/run/libvirt", SYSTEM_UID, SYSTEM_UID);
-
-    if (__init_daemon_priv(PU_RESETGROUPS | PU_CLEARLIMITSET,
-                           SYSTEM_UID, SYSTEM_UID, PRIV_XVM_CONTROL, NULL)) {
-        VIR_ERROR(_("additional privileges are required"));
-        return -1;
-    }
-
-    if (priv_set(PRIV_OFF, PRIV_ALLSETS, PRIV_FILE_LINK_ANY, PRIV_PROC_INFO,
-                 PRIV_PROC_SESSION, PRIV_PROC_EXEC, PRIV_PROC_FORK, NULL)) {
-        VIR_ERROR(_("failed to set reduced privileges"));
-        return -1;
-    }
-
-    return 0;
-}
-#else
-# define daemonSetupPrivs() 0
-#endif
-
 
 static void daemonShutdownHandler(virNetDaemonPtr dmn,
                                   siginfo_t *sig ATTRIBUTE_UNUSED,
@@ -1336,15 +1312,6 @@ int main(int argc, char **argv) {
         goto cleanup;
     }
 
-    /* Beyond this point, nothing should rely on using
-     * getuid/geteuid() == 0, for privilege level checks.
-     */
-    VIR_DEBUG("Dropping privileges (if required)");
-    if (daemonSetupPrivs() < 0) {
-        ret = VIR_DAEMON_ERR_PRIVS;
-        goto cleanup;
-    }
-
     daemonInitialize();
 
     remoteProcs[REMOTE_PROC_AUTH_LIST].needAuth = false;
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index f765c09416..34e5ced0a9 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1350,7 +1350,6 @@ remoteConnectOpen(virConnectPtr conn,
      */
     if (!conn->uri) {
         VIR_DEBUG("Auto-probe remote URI");
-#ifndef __sun
         if (geteuid() > 0) {
             VIR_DEBUG("Auto-spawn user daemon instance");
             rflags |= VIR_DRV_OPEN_REMOTE_USER;
@@ -1359,7 +1358,6 @@ remoteConnectOpen(virConnectPtr conn,
                  STRNEQ(autostart, "0")))
                 rflags |= VIR_DRV_OPEN_REMOTE_AUTOSTART;
         }
-#endif
     }
 
     ret = doRemoteOpen(conn, priv, auth, conf, rflags);
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] remote: remove some __sun conditionals
Posted by Andrea Bolognani 6 years ago
On Fri, 2018-03-23 at 11:02 +0000, Daniel P. Berrangé wrote:
> The libvirtd daemon has some arbitrary logic to drop privileges, but
> only on Solaris platforms. This was added during Xen days, when Xen was
> the only driver running in libvirtd. There's no expectation or testing
> that this works with the new libxl stack, nor whether dropping
> privileges breaks any of the secondary drivers. Finally, we'll be
> splitting drivers out into their own independant daemons, so this won't
> be applicable to libvirtd in future anyway.
> 
> The remote driver client meanwhile arbitrarily disables daemon
> auto-spawn when connecting as non-root, breaking a key feature of
> libvirt unprivileged connections.
> 
> Since we've not had any contributions for Solaris since circa 2012
> and we don't do any CI testing we should consider this platform
> unmaintained and thus reasonable to remove this cruft. If someone steps
> forward to maintain Solaris again, this code would need re-evaluating to
> come up with something more targetted.
> 
> There's various __sun conditionals in the Xen driver code, but those are
> not touched. This is all for the legacy Xen driver, which will be
> entirely removed at some point in future, so not benefit to hacking out
> just the Solaris parts.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/remote/remote_daemon.c | 33 ---------------------------------
>  src/remote/remote_driver.c |  2 --
>  2 files changed, 35 deletions(-)

Looks reasonable enough.

Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

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