Convert polkit code to use DBus API instead of CLI helper

Spawning the pkcheck program every time a permission check is
required is hugely expensive on CPU. The pkcheck program is just
a dumb wrapper for the DBus API, so rewrite the code to use the
DBus API directly. This also simplifies error handling a bit.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

cfg.mk

@@ -1143,3 +1143,6 @@ exclude_file_name_regexp--sc_prohibit_mixed_case_abbreviations = \
 
 exclude_file_name_regexp--sc_prohibit_empty_first_line = \
   ^(README|daemon/THREADS\.txt|src/esx/README|docs/library.xen|tests/vmwareverdata/fusion-5.0.3.txt|tests/nodeinfodata/linux-raspberrypi/cpu/offline)$$
+
+exclude_file_name_regexp--sc_prohibit_useless_translation = \
+  ^tests/virpolkittest.c

po/POTFILES.in

@@ -236,6 +236,7 @@ src/xenapi/xenapi_utils.c
 src/xenconfig/xen_common.c
 src/xenconfig/xen_sxpr.c
 src/xenconfig/xen_xm.c
+tests/virpolkittest.c
 tools/libvirt-guests.sh.in
 tools/virsh.c
 tools/virsh.h

src/util/virpolkit.c

@@ -60,84 +60,76 @@ int virPolkitCheckAuth(const char *actionid,
                        const char **details,
                        bool allowInteraction)
 {
-    int status = -1;
-    bool authdismissed = 0;
-    bool supportsuid = 0;
-    char *pkout = NULL;
-    virCommandPtr cmd = NULL;
+    DBusConnection *sysbus;
+    DBusMessage *reply = NULL;
+    char **retdetails = NULL;
+    size_t nretdetails = 0;
+    int is_authorized; /* var-args requires int not bool */
+    int is_challenge; /* var-args requires int not bool */
+    bool is_dismissed = false;
+    size_t i;
     int ret = -1;
-    static bool polkitInsecureWarned = false;
-
-    VIR_DEBUG("Checking PID %lld UID %d startTime %llu",
-              (long long)pid, (int)uid, startTime);
 
-    if (startTime == 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Start time is required for polkit auth"));
-        return -1;
-    }
-
-    cmd = virCommandNewArgList(PKCHECK_PATH, "--action-id", actionid, NULL);
-    virCommandSetOutputBuffer(cmd, &pkout);
-    virCommandSetErrorBuffer(cmd, &pkout);
-
-    virCommandAddArg(cmd, "--process");
-# ifdef PKCHECK_SUPPORTS_UID
-    supportsuid = 1;
-# endif
-    if (supportsuid) {
-        virCommandAddArgFormat(cmd, "%lld,%llu,%lu",
-                               (long long)pid, startTime, (unsigned long)uid);
-    } 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)pid, startTime);
-    }
-    if (allowInteraction)
-        virCommandAddArg(cmd, "--allow-user-interaction");
+    if (!(sysbus = virDBusGetSystemBus()))
+        goto cleanup;
 
-    while (details && details[0] && details[1]) {
-        virCommandAddArgList(cmd, "--detail", details[0], details[1], NULL);
-        details += 2;
-    }
+    VIR_INFO("Checking PID %lld running as %d",
+             (long long) pid, uid);
 
-    if (virCommandRun(cmd, &status) < 0)
+    if (virDBusCallMethod(sysbus,
+                          &reply,
+                          NULL,
+                          "org.freedesktop.PolicyKit1",
+                          "/org/freedesktop/PolicyKit1/Authority",
+                          "org.freedesktop.PolicyKit1.Authority",
+                          "CheckAuthorization",
+                          "(sa{sv})sa&{ss}us",
+                          "unix-process",
+                          3,
+                          "pid", "u", (unsigned int)pid,
+                          "start-time", "t", startTime,
+                          "uid", "i", (int)uid,
+                          actionid,
+                          virStringListLen(details) / 2,
+                          details,
+                          allowInteraction,
+                          "" /* cancellation ID */) < 0)
         goto cleanup;
 
-    authdismissed = (pkout && strstr(pkout, "dismissed=true"));
-    if (status != 0) {
-        char *tmp = virProcessTranslateStatus(status);
-        VIR_DEBUG("Policy kit denied action %s from pid %lld, uid %d: %s",
-                  actionid, (long long)pid, (int)uid, NULLSTR(tmp));
-        VIR_FREE(tmp);
-        ret = -2;
+    if (virDBusMessageRead(reply,
+                           "(bba&{ss})",
+                           &is_authorized,
+                           &is_challenge,
+                           &nretdetails,
+                           &retdetails) < 0)
         goto cleanup;
-    }
 
-    VIR_DEBUG("Policy allowed action %s from pid %lld, uid %d",
-              actionid, (long long)pid, (int)uid);
+    for (i = 0; i < (nretdetails / 2); i++) {
+        if (STREQ(retdetails[(i * 2)], "polkit.dismissed") &&
+            STREQ(retdetails[(i * 2) + 1], "true"))
+            is_dismissed = true;
+    }
 
-    ret = 0;
+    VIR_DEBUG("is auth %d  is challenge %d",
+              is_authorized, is_challenge);
 
- cleanup:
-    if (ret < 0) {
-        virResetLastError();
-
-        if (authdismissed) {
+    if (is_authorized) {
+        ret = 0;
+    } else {
+        ret = -2;
+        if (is_dismissed)
             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"));
-        }
+                           _("user cancelled authentication process"));
+        else if (is_challenge)
+            virReportError(VIR_ERR_AUTH_FAILED, "%s",
+                           _("no agent is available to authenticate"));
+        else
+            virReportError(VIR_ERR_AUTH_FAILED, "%s",
+                           _("access denied by policy"));
     }
 
-    virCommandFree(cmd);
-    VIR_FREE(pkout);
+ cleanup:
+    virStringFreeListCount(retdetails, nretdetails);
     return ret;
 }
 

tests/Makefile.am

@@ -197,7 +197,11 @@ endif WITH_LIBVIRTD
 
 if WITH_DBUS
 test_programs += virdbustest \
-                 virsystemdtest
+                 virsystemdtest \
+                 $(NULL)
+if WITH_POLKIT1
+test_programs += virpolkittest
+endif WITH_POLKIT1
 endif WITH_DBUS
 
 if WITH_SECDRIVER_SELINUX
@@ -1008,6 +1012,11 @@ virmockdbus_la_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS)
 virmockdbus_la_LDFLAGS = -module -avoid-version \
         -rpath /evil/libtool/hack/to/force/shared/lib/creation
 
+virpolkittest_SOURCES = \
+	virpolkittest.c testutils.h testutils.c
+virpolkittest_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS)
+virpolkittest_LDADD = $(LDADDS) $(DBUS_LIBS)
+
 virsystemdtest_SOURCES = \
 	virsystemdtest.c testutils.h testutils.c
 virsystemdtest_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS)

tests/virpolkittest.c

+/*
+ * Copyright (C) 2013, 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Daniel P. Berrange <berrange@redhat.com>
+ */
+
+#include <config.h>
+
+#include "testutils.h"
+
+#if defined(WITH_DBUS) && defined(__linux__)
+
+# include <stdlib.h>
+# include <dbus/dbus.h>
+
+# include "virpolkit.h"
+# include "virdbus.h"
+# include "virlog.h"
+# include "virmock.h"
+# define VIR_FROM_THIS VIR_FROM_NONE
+
+VIR_LOG_INIT("tests.systemdtest");
+
+/* Some interesting numbers */
+# define THE_PID 1458
+# define THE_TIME 11011000001
+# define THE_UID 1729
+
+VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
+                       DBusMessage *,
+                       DBusConnection *, connection,
+                       DBusMessage *, message,
+                       int, timeout_milliseconds,
+                       DBusError *, error)
+{
+    DBusMessage *reply = NULL;
+    const char *service = dbus_message_get_destination(message);
+    const char *member = dbus_message_get_member(message);
+
+    VIR_MOCK_REAL_INIT(dbus_connection_send_with_reply_and_block);
+
+    if (STREQ(service, "org.freedesktop.PolicyKit1") &&
+        STREQ(member, "CheckAuthorization")) {
+        char *type;
+        char *pidkey;
+        unsigned int pidval;
+        char *timekey;
+        unsigned long long timeval;
+        char *uidkey;
+        int uidval;
+        char *actionid;
+        char **details;
+        size_t detailslen;
+        int allowInteraction;
+        char *cancellationId;
+        const char **retdetails = NULL;
+        size_t retdetailslen = 0;
+        const char *retdetailscancelled[] = {
+            "polkit.dismissed", "true",
+        };
+        int is_authorized = 1;
+        int is_challenge = 0;
+
+        if (virDBusMessageRead(message,
+                               "(sa{sv})sa&{ss}us",
+                               &type,
+                               3,
+                               &pidkey, "u", &pidval,
+                               &timekey, "t", &timeval,
+                               &uidkey, "i", &uidval,
+                               &actionid,
+                               &detailslen,
+                               &details,
+                               &allowInteraction,
+                               &cancellationId) < 0)
+            goto error;
+
+        if (STREQ(actionid, "org.libvirt.test.success")) {
+            is_authorized = 1;
+            is_challenge = 0;
+        } else if (STREQ(actionid, "org.libvirt.test.challenge")) {
+            is_authorized = 0;
+            is_challenge = 1;
+        } else if (STREQ(actionid, "org.libvirt.test.cancelled")) {
+            is_authorized = 0;
+            is_challenge = 0;
+            retdetails = retdetailscancelled;
+            retdetailslen = ARRAY_CARDINALITY(retdetailscancelled) / 2;
+        } else if (STREQ(actionid, "org.libvirt.test.details")) {
+            size_t i;
+            is_authorized = 0;
+            is_challenge = 0;
+            for (i = 0; i < detailslen / 2; i++) {
+                if (STREQ(details[i * 2],
+                          "org.libvirt.test.person") &&
+                    STREQ(details[(i * 2) + 1],
+                          "Fred")) {
+                    is_authorized = 1;
+                    is_challenge = 0;
+                }
+            }
+        } else {
+            is_authorized = 0;
+            is_challenge = 0;
+        }
+
+        VIR_FREE(type);
+        VIR_FREE(pidkey);
+        VIR_FREE(timekey);
+        VIR_FREE(uidkey);
+        VIR_FREE(actionid);
+        VIR_FREE(cancellationId);
+        virStringFreeListCount(details, detailslen);
+
+        if (virDBusCreateReply(&reply,
+                               "(bba&{ss})",
+                               is_authorized,
+                               is_challenge,
+                               retdetailslen,
+                               retdetails) < 0)
+            goto error;
+    } else {
+        reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
+    }
+
+    return reply;
+
+ error:
+    dbus_message_unref(reply);
+    return NULL;
+}
+
+
+
+static int testPolkitAuthSuccess(const void *opaque ATTRIBUTE_UNUSED)
+{
+    int ret = -1;
+
+    if (virPolkitCheckAuth("org.libvirt.test.success",
+                           THE_PID,
+                           THE_TIME,
+                           THE_UID,
+                           NULL,
+                           true) < 0)
+        goto cleanup;
+
+    ret = 0;
+
+ cleanup:
+    return ret;
+}
+
+
+static int testPolkitAuthDenied(const void *opaque ATTRIBUTE_UNUSED)
+{
+    int ret = -1;
+    int rv;
+    virErrorPtr err;
+
+    rv = virPolkitCheckAuth("org.libvirt.test.deny",
+                            THE_PID,
+                            THE_TIME,
+                            THE_UID,
+                            NULL,
+                            true);
+
+    if (rv == 0) {
+        fprintf(stderr, "Unexpected auth success\n");
+        goto cleanup;
+    } else if (rv != -2) {
+        goto cleanup;
+    }
+
+    err = virGetLastError();
+    if (!err || !strstr(err->message,
+                        _("access denied by policy"))) {
+        fprintf(stderr, "Incorrect error response\n");
+        goto cleanup;
+    }
+
+    ret = 0;
+
+ cleanup:
+    return ret;
+}
+
+
+static int testPolkitAuthChallenge(const void *opaque ATTRIBUTE_UNUSED)
+{
+    int ret = -1;
+    int rv;
+    virErrorPtr err;
+
+    rv = virPolkitCheckAuth("org.libvirt.test.challenge",
+                            THE_PID,
+                            THE_TIME,
+                            THE_UID,
+                            NULL,
+                            true);
+
+    if (rv == 0) {
+        fprintf(stderr, "Unexpected auth success\n");
+        goto cleanup;
+    } else if (rv != -2) {
+        goto cleanup;
+    }
+
+    err = virGetLastError();
+    if (!err || !strstr(err->message,
+                       _("no agent is available to authenticate"))) {
+        fprintf(stderr, "Incorrect error response\n");
+        goto cleanup;
+    }
+
+    ret = 0;
+
+ cleanup:
+    return ret;
+}
+
+
+static int testPolkitAuthCancelled(const void *opaque ATTRIBUTE_UNUSED)
+{
+    int ret = -1;
+    int rv;
+    virErrorPtr err;
+
+    rv = virPolkitCheckAuth("org.libvirt.test.cancelled",
+                            THE_PID,
+                            THE_TIME,
+                            THE_UID,
+                            NULL,
+                            true);
+
+    if (rv == 0) {
+        fprintf(stderr, "Unexpected auth success\n");
+        goto cleanup;
+    } else if (rv != -2) {
+        goto cleanup;
+    }
+
+    err = virGetLastError();
+    if (!err || !strstr(err->message,
+                       _("user cancelled authentication process"))) {
+        fprintf(stderr, "Incorrect error response\n");
+        goto cleanup;
+    }
+
+    ret = 0;
+
+ cleanup:
+    return ret;
+}
+
+
+static int testPolkitAuthDetailsSuccess(const void *opaque ATTRIBUTE_UNUSED)
+{
+    int ret = -1;
+    const char *details[] = {
+        "org.libvirt.test.person", "Fred",
+        NULL,
+    };
+
+    if (virPolkitCheckAuth("org.libvirt.test.details",
+                           THE_PID,
+                           THE_TIME,
+                           THE_UID,
+                           details,
+                           true) < 0)
+        goto cleanup;
+
+    ret = 0;
+
+ cleanup:
+    return ret;
+}
+
+
+static int testPolkitAuthDetailsDenied(const void *opaque ATTRIBUTE_UNUSED)
+{
+    int ret = -1;
+    int rv;
+    virErrorPtr err;
+    const char *details[] = {
+        "org.libvirt.test.person", "Joe",
+        NULL,
+    };
+
+    rv = virPolkitCheckAuth("org.libvirt.test.details",
+                            THE_PID,
+                            THE_TIME,
+                            THE_UID,
+                            details,
+                            true);
+
+    if (rv == 0) {
+        fprintf(stderr, "Unexpected auth success\n");
+        goto cleanup;
+    } else if (rv != -2) {
+        goto cleanup;
+    }
+
+    err = virGetLastError();
+    if (!err || !strstr(err->message,
+                        _("access denied by policy"))) {
+        fprintf(stderr, "Incorrect error response\n");
+        goto cleanup;
+    }
+
+    ret = 0;
+
+ cleanup:
+    return ret;
+}
+
+
+static int
+mymain(void)
+{
+    int ret = 0;
+
+    if (virtTestRun("Polkit auth success ", testPolkitAuthSuccess, NULL) < 0)
+        ret = -1;
+    if (virtTestRun("Polkit auth deny ", testPolkitAuthDenied, NULL) < 0)
+        ret = -1;
+    if (virtTestRun("Polkit auth challenge ", testPolkitAuthChallenge, NULL) < 0)
+        ret = -1;
+    if (virtTestRun("Polkit auth cancel ", testPolkitAuthCancelled, NULL) < 0)
+        ret = -1;
+    if (virtTestRun("Polkit auth details success ", testPolkitAuthDetailsSuccess, NULL) < 0)
+        ret = -1;
+    if (virtTestRun("Polkit auth details deny ", testPolkitAuthDetailsDenied, NULL) < 0)
+        ret = -1;
+
+    return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virmockdbus.so")
+
+#else /* ! (WITH_DBUS && __linux__) */
+int
+main(void)
+{
+    return EXIT_AM_SKIP;
+}
+#endif /* ! WITH_DBUS */