Convert remote daemon & acl code to use polkit API

Convert the remote daemon auth check and the access control
code to use the common polkit API for checking auth.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

daemon/Makefile.am

@@ -141,7 +141,7 @@ libvirtd_SOURCES = $(DAEMON_SOURCES)
 #-D_XOPEN_SOURCE=600 -D_XOPEN_SOURCE_EXTENDED=1 -D_POSIX_C_SOURCE=199506L
 libvirtd_CFLAGS = \
 	$(LIBXML_CFLAGS) $(GNUTLS_CFLAGS) $(SASL_CFLAGS) \
-	$(XDR_CFLAGS) $(POLKIT_CFLAGS) $(DBUS_CFLAGS) $(LIBNL_CFLAGS) \
+	$(XDR_CFLAGS) $(DBUS_CFLAGS) $(LIBNL_CFLAGS) \
 	$(WARN_CFLAGS) $(PIE_CFLAGS) \
 	$(COVERAGE_CFLAGS) \
 	-DQEMUD_PID_FILE="\"$(QEMUD_PID_FILE)\""
@@ -158,7 +158,6 @@ libvirtd_LDADD =					\
 	$(GNUTLS_LIBS)					\
 	$(SASL_LIBS)					\
 	$(DBUS_LIBS)					\
-	$(POLKIT_LIBS)					\
 	$(LIBNL_LIBS)
 
 if WITH_DTRACE_PROBES

daemon/remote.c

@@ -24,11 +24,6 @@
 
 #include "virerror.h"
 
-#if WITH_POLKIT0
-# include <polkit/polkit.h>
-# include <polkit-dbus/polkit-dbus.h>
-#endif
-
 #include "remote.h"
 #include "libvirtd.h"
 #include "libvirt_internal.h"
@@ -55,6 +50,7 @@
 #include "virprobe.h"
 #include "viraccessapicheck.h"
 #include "viraccessapicheckqemu.h"
+#include "virpolkit.h"
 
 #define VIR_FROM_THIS VIR_FROM_RPC
 
@@ -3230,7 +3226,6 @@ remoteDispatchAuthSaslStep(virNetServerPtr server ATTRIBUTE_UNUSED,
 
 
 
-#if WITH_POLKIT1
 static int
 remoteDispatchAuthPolkit(virNetServerPtr server,
                          virNetServerClientPtr client,
@@ -3243,26 +3238,16 @@ remoteDispatchAuthPolkit(virNetServerPtr server,
     uid_t callerUid = -1;
     unsigned long long timestamp;
     const char *action;
-    int status = -1;
     char *ident = NULL;
-    bool authdismissed = 0;
-    char *pkout = NULL;
     struct daemonClientPrivate *priv =
         virNetServerClientGetPrivateData(client);
-    virCommandPtr cmd = NULL;
+    int rv;
-# ifndef PKCHECK_SUPPORTS_UID
-    static bool polkitInsecureWarned;
-# endif
 
     virMutexLock(&priv->lock);
     action = virNetServerClientGetReadonly(client) ?
         "org.libvirt.unix.monitor" :
         "org.libvirt.unix.manage";
 
-    cmd = virCommandNewArgList(PKCHECK_PATH, "--action-id", action, NULL);
-    virCommandSetOutputBuffer(cmd, &pkout);
-    virCommandSetErrorBuffer(cmd, &pkout);
-
     VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(client));
     if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_POLKIT) {
         VIR_ERROR(_("client tried invalid PolicyKit init request"));
@@ -3283,38 +3268,17 @@ remoteDispatchAuthPolkit(virNetServerPtr server,
     VIR_INFO("Checking PID %lld running as %d",
              (long long) callerPid, callerUid);
 
-    virCommandAddArg(cmd, "--process");
+    rv = virPolkitCheckAuth(action,
-
+                            callerPid,
-# ifdef PKCHECK_SUPPORTS_UID
+                            timestamp,
-    virCommandAddArgFormat(cmd, "%lld,%llu,%lu",
+                            callerUid,
-                           (long long) callerPid,
+                            NULL,
-                           timestamp,
+                            true);
-                           (unsigned long) callerUid);
+    if (rv == -1)
-# else
-    if (!polkitInsecureWarned) {
-        VIR_WARN("No support for caller UID with pkcheck. "
-                 "This deployment is known to be insecure.");
-        polkitInsecureWarned = true;
-    }
-    virCommandAddArgFormat(cmd, "%lld,%llu", (long long) callerPid, timestamp);
-# endif
-
-    virCommandAddArg(cmd, "--allow-user-interaction");
-
-    if (virAsprintf(&ident, "pid:%lld,uid:%d",
-                    (long long) callerPid, callerUid) < 0)
         goto authfail;
-
+    else if (rv == -2)
-    if (virCommandRun(cmd, &status) < 0)
-        goto authfail;
-
-    authdismissed = (pkout && strstr(pkout, "dismissed=true"));
-    if (status != 0) {
-        VIR_ERROR(_("Policy kit denied action %s from pid %lld, uid %d "
-                    "with status %d"),
-                  action, (long long) callerPid, callerUid, status);
         goto authdeny;
-    }
+
     PROBE(RPC_SERVER_CLIENT_AUTH_ALLOW,
           "client=%p auth=%d identity=%s",
           client, REMOTE_AUTH_POLKIT, ident);
@@ -3325,27 +3289,10 @@ remoteDispatchAuthPolkit(virNetServerPtr server,
     virNetServerClientSetAuth(client, 0);
     virNetServerTrackCompletedAuth(server);
     virMutexUnlock(&priv->lock);
-    virCommandFree(cmd);
-    VIR_FREE(pkout);
-    VIR_FREE(ident);
 
     return 0;
 
  error:
-    virCommandFree(cmd);
-    VIR_FREE(ident);
-    virResetLastError();
-
-    if (authdismissed) {
-        virReportError(VIR_ERR_AUTH_CANCELLED, "%s",
-                       _("authentication cancelled by user"));
-    } else if (pkout && *pkout) {
-        virReportError(VIR_ERR_AUTH_FAILED, _("polkit: %s"), pkout);
-    } else {
-        virReportError(VIR_ERR_AUTH_FAILED, "%s", _("authentication failed"));
-    }
-
-    VIR_FREE(pkout);
     virNetMessageSaveError(rerr);
     virMutexUnlock(&priv->lock);
     return -1;
@@ -3362,166 +3309,6 @@ remoteDispatchAuthPolkit(virNetServerPtr server,
           client, REMOTE_AUTH_POLKIT, ident);
     goto error;
 }
-#elif WITH_POLKIT0
-static int
-remoteDispatchAuthPolkit(virNetServerPtr server,
-                         virNetServerClientPtr client,
-                         virNetMessagePtr msg ATTRIBUTE_UNUSED,
-                         virNetMessageErrorPtr rerr,
-                         remote_auth_polkit_ret *ret)
-{
-    pid_t callerPid;
-    gid_t callerGid;
-    uid_t callerUid;
-    PolKitCaller *pkcaller = NULL;
-    PolKitAction *pkaction = NULL;
-    PolKitContext *pkcontext = NULL;
-    PolKitError *pkerr = NULL;
-    PolKitResult pkresult;
-    DBusError err;
-    const char *action;
-    char *ident = NULL;
-    struct daemonClientPrivate *priv =
-        virNetServerClientGetPrivateData(client);
-    DBusConnection *sysbus;
-    unsigned long long timestamp;
-
-    virMutexLock(&priv->lock);
-
-    action = virNetServerClientGetReadonly(client) ?
-        "org.libvirt.unix.monitor" :
-        "org.libvirt.unix.manage";
-
-    VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(client));
-    if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_POLKIT) {
-        VIR_ERROR(_("client tried invalid PolicyKit init request"));
-        goto authfail;
-    }
-
-    if (virNetServerClientGetUNIXIdentity(client, &callerUid, &callerGid,
-                                          &callerPid, &timestamp) < 0) {
-        VIR_ERROR(_("cannot get peer socket identity"));
-        goto authfail;
-    }
-
-    if (virAsprintf(&ident, "pid:%lld,uid:%d",
-                    (long long) callerPid, callerUid) < 0)
-        goto authfail;
-
-    if (!(sysbus = virDBusGetSystemBus()))
-        goto authfail;
-
-    VIR_INFO("Checking PID %lld running as %d",
-             (long long) callerPid, callerUid);
-    dbus_error_init(&err);
-    if (!(pkcaller = polkit_caller_new_from_pid(sysbus,
-                                                callerPid, &err))) {
-        VIR_ERROR(_("Failed to lookup policy kit caller: %s"), err.message);
-        dbus_error_free(&err);
-        goto authfail;
-    }
-
-    if (!(pkaction = polkit_action_new())) {
-        char ebuf[1024];
-        VIR_ERROR(_("Failed to create polkit action %s"),
-                  virStrerror(errno, ebuf, sizeof(ebuf)));
-        polkit_caller_unref(pkcaller);
-        goto authfail;
-    }
-    polkit_action_set_action_id(pkaction, action);
-
-    if (!(pkcontext = polkit_context_new()) ||
-        !polkit_context_init(pkcontext, &pkerr)) {
-        char ebuf[1024];
-        VIR_ERROR(_("Failed to create polkit context %s"),
-                  (pkerr ? polkit_error_get_error_message(pkerr)
-                   : virStrerror(errno, ebuf, sizeof(ebuf))));
-        if (pkerr)
-            polkit_error_free(pkerr);
-        polkit_caller_unref(pkcaller);
-        polkit_action_unref(pkaction);
-        dbus_error_free(&err);
-        goto authfail;
-    }
-
-# if HAVE_POLKIT_CONTEXT_IS_CALLER_AUTHORIZED
-    pkresult = polkit_context_is_caller_authorized(pkcontext,
-                                                   pkaction,
-                                                   pkcaller,
-                                                   0,
-                                                   &pkerr);
-    if (pkerr && polkit_error_is_set(pkerr)) {
-        VIR_ERROR(_("Policy kit failed to check authorization %d %s"),
-                  polkit_error_get_error_code(pkerr),
-                  polkit_error_get_error_message(pkerr));
-        goto authfail;
-    }
-# else
-    pkresult = polkit_context_can_caller_do_action(pkcontext,
-                                                   pkaction,
-                                                   pkcaller);
-# endif
-    polkit_context_unref(pkcontext);
-    polkit_caller_unref(pkcaller);
-    polkit_action_unref(pkaction);
-    if (pkresult != POLKIT_RESULT_YES) {
-        VIR_ERROR(_("Policy kit denied action %s from pid %lld, uid %d, result: %s"),
-                  action, (long long) callerPid, callerUid,
-                  polkit_result_to_string_representation(pkresult));
-        goto authdeny;
-    }
-    PROBE(RPC_SERVER_CLIENT_AUTH_ALLOW,
-          "client=%p auth=%d identity=%s",
-          client, REMOTE_AUTH_POLKIT, ident);
-    VIR_INFO("Policy allowed action %s from pid %lld, uid %d, result %s",
-             action, (long long) callerPid, callerUid,
-             polkit_result_to_string_representation(pkresult));
-    ret->complete = 1;
-
-    virNetServerClientSetAuth(client, 0);
-    virNetServerTrackCompletedAuth(server);
-    virMutexUnlock(&priv->lock);
-    VIR_FREE(ident);
-    return 0;
-
- error:
-    VIR_FREE(ident);
-    virResetLastError();
-    virReportError(VIR_ERR_AUTH_FAILED, "%s",
-                   _("authentication failed"));
-    virNetMessageSaveError(rerr);
-    virMutexUnlock(&priv->lock);
-    return -1;
-
- authfail:
-    PROBE(RPC_SERVER_CLIENT_AUTH_FAIL,
-          "client=%p auth=%d",
-          client, REMOTE_AUTH_POLKIT);
-    goto error;
-
- authdeny:
-    PROBE(RPC_SERVER_CLIENT_AUTH_DENY,
-          "client=%p auth=%d identity=%s",
-          client, REMOTE_AUTH_POLKIT, ident);
-    goto error;
-}
-
-#else /* !WITH_POLKIT0 & !HAVE_POLKIT1*/
-
-static int
-remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
-                         virNetServerClientPtr client ATTRIBUTE_UNUSED,
-                         virNetMessagePtr msg ATTRIBUTE_UNUSED,
-                         virNetMessageErrorPtr rerr,
-                         remote_auth_polkit_ret *ret ATTRIBUTE_UNUSED)
-{
-    VIR_ERROR(_("client tried unsupported PolicyKit init request"));
-    virReportError(VIR_ERR_AUTH_FAILED, "%s",
-                   _("authentication failed"));
-    virNetMessageSaveError(rerr);
-    return -1;
-}
-#endif /* WITH_POLKIT1 */
 
 
 /***************************************************************

src/Makefile.am

@@ -2486,13 +2486,11 @@ libvirt_net_rpc_server_la_CFLAGS = \
 			$(AVAHI_CFLAGS) \
 			$(DBUS_CFLAGS) \
 			$(XDR_CFLAGS) \
-			$(AM_CFLAGS) \
+			$(AM_CFLAGS)
-			$(POLKIT_CFLAGS)
 libvirt_net_rpc_server_la_LDFLAGS = \
 			$(AM_LDFLAGS) \
 			$(AVAHI_LIBS) \
 			$(DBUS_LIBS) \
-			$(POLKIT_LIBS) \
 			$(CYGWIN_EXTRA_LDFLAGS) \
 			$(MINGW_EXTRA_LDFLAGS)
 libvirt_net_rpc_server_la_LIBADD = \

src/access/viraccessdriverpolkit.c

@@ -26,6 +26,7 @@
 #include "virlog.h"
 #include "virprocess.h"
 #include "virerror.h"
+#include "virpolkit.h"
 #include "virstring.h"
 
 #define VIR_FROM_THIS VIR_FROM_ACCESS
@@ -71,29 +72,26 @@ virAccessDriverPolkitFormatAction(const char *typename,
 }
 
 
-static char *
+static int
-virAccessDriverPolkitFormatProcess(const char *actionid)
+virAccessDriverPolkitGetCaller(const char *actionid,
+                               pid_t *pid,
+                               unsigned long long *startTime,
+                               uid_t *uid)
 {
     virIdentityPtr identity = virIdentityGetCurrent();
-    pid_t pid;
+    int ret = -1;
-    unsigned long long startTime;
-    uid_t uid;
-    char *ret = NULL;
-#ifndef PKCHECK_SUPPORTS_UID
-    static bool polkitInsecureWarned;
-#endif
 
     if (!identity) {
         virAccessError(VIR_ERR_ACCESS_DENIED,
                        _("Policy kit denied action %s from <anonymous>"),
                        actionid);
-        return NULL;
+        return -1;
     }
-    if (virIdentityGetUNIXProcessID(identity, &pid) < 0)
+    if (virIdentityGetUNIXProcessID(identity, pid) < 0)
         goto cleanup;
-    if (virIdentityGetUNIXProcessTime(identity, &startTime) < 0)
+    if (virIdentityGetUNIXProcessTime(identity, startTime) < 0)
         goto cleanup;
-    if (virIdentityGetUNIXUserID(identity, &uid) < 0)
+    if (virIdentityGetUNIXUserID(identity, uid) < 0)
         goto cleanup;
 
     if (!pid) {
@@ -101,25 +99,14 @@ virAccessDriverPolkitFormatProcess(const char *actionid)
                        _("No UNIX process ID available"));
         goto cleanup;
     }
-    if (!startTime) {
-        virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("No UNIX process start time available"));
-        goto cleanup;
-    }
 
-#ifdef PKCHECK_SUPPORTS_UID
+    if (virIdentityGetUNIXProcessTime(identity, startTime) < 0)
-    if (virAsprintf(&ret, "%llu,%llu,%llu",
-                    (unsigned long long)pid, startTime, (unsigned long long)uid) < 0)
         goto cleanup;
-#else
+
-    if (!polkitInsecureWarned) {
+    if (virIdentityGetUNIXUserID(identity, uid) < 0)
-        VIR_WARN("No support for caller UID with pkcheck. This deployment is known to be insecure.");
-        polkitInsecureWarned = true;
-    }
-    if (virAsprintf(&ret, "%llu,%llu",
-                    (unsigned long long)pid, startTime) < 0)
         goto cleanup;
-#endif
+
+    ret = 0;
 
  cleanup:
     virObjectUnref(identity);
@@ -134,53 +121,43 @@ virAccessDriverPolkitCheck(virAccessManagerPtr manager ATTRIBUTE_UNUSED,
                            const char **attrs)
 {
     char *actionid = NULL;
-    char *process = NULL;
-    virCommandPtr cmd = NULL;
-    int status;
     int ret = -1;
+    pid_t pid;
+    uid_t uid;
+    unsigned long long startTime;
+    int rv;
 
     if (!(actionid = virAccessDriverPolkitFormatAction(typename, permname)))
         goto cleanup;
 
-    if (!(process = virAccessDriverPolkitFormatProcess(actionid)))
+    if (virAccessDriverPolkitGetCaller(actionid,
+                                       &pid,
+                                       &startTime,
+                                       &uid) < 0)
         goto cleanup;
 
-    VIR_DEBUG("Check action '%s' for process '%s'", actionid, process);
+    VIR_DEBUG("Check action '%s' for process '%d' time %lld uid %d",
+              actionid, pid, startTime, uid);
 
-    cmd = virCommandNewArgList(PKCHECK_PATH,
+    rv = virPolkitCheckAuth(actionid,
-                               "--action-id", actionid,
+                            pid,
-                               "--process", process,
+                            startTime,
-                               NULL);
+                            uid,
+                            attrs,
+                            false);
 
-    while (attrs && attrs[0] && attrs[1]) {
+    if (rv == 0) {
-        virCommandAddArgList(cmd, "--detail", attrs[0], attrs[1], NULL);
-        attrs += 2;
-    }
-
-    if (virCommandRun(cmd, &status) < 0)
-        goto cleanup;
-
-    if (status == 0) {
         ret = 1; /* Allowed */
     } else {
-        if (status == 1 ||
+        if (rv == -2) {
-            status == 2 ||
-            status == 3) {
             ret = 0; /* Denied */
         } else {
             ret = -1; /* Error */
-            virAccessError(VIR_ERR_ACCESS_DENIED,
-                           _("Policy kit denied action %s from %s: "
-                             "exit status %d"),
-                           actionid, process, status);
         }
-        goto cleanup;
     }
 
  cleanup:
-    virCommandFree(cmd);
     VIR_FREE(actionid);
-    VIR_FREE(process);
     return ret;
 }