[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] PATCH: Support PolicyKit 1.0



In the seriously annoying way of things, the newest PolicyKit in Fedora 12
has been completely re-written from scratch with a totally incompatible
application facing API.  Conceptually it is still pretty similar though,
with the exception that client applications no longer need to explicitly
launch an auth dialog - that's done out-of-band by policykit itself.

This patch adjusts libvirtd to the new API, and removes the libvirt client
side code that spawned the auth helper. On the libvirtd side avoid their
APIs and instead spawn an external auth checking program 'pkcheck', which
returns 0 on success, non-0 for denial.

In the final annoying bit, the XML format for the policy has remained with
exactly the same DTD version, except that it has quietly changed the allowed
values for some attributes in an incompatible manner.  So I have to add a
new policy file too.

NB, it may not look like we've changed the client side, but we have, since
the #ifdef for the external auth agent is no longer set.

Tested with old policy kit, and new policykit, and with none at all.

 b/configure.in            |   73 +++++++++++++++++++++++++++--------------
 b/qemud/Makefile.am       |   11 +++++-
 b/qemud/libvirtd.policy-0 |   42 +++++++++++++++++++++++
 b/qemud/libvirtd.policy-1 |   42 +++++++++++++++++++++++
 b/qemud/qemud.c           |    4 +-
 b/qemud/qemud.h           |    4 +-
 b/qemud/remote.c          |   81 +++++++++++++++++++++++++++++++++++++++++++---
 qemud/libvirtd.policy     |   42 -----------------------
 8 files changed, 223 insertions(+), 76 deletions(-)

Regards,
Daniel

diff --git a/configure.in b/configure.in
index b905b23..77d6c9e 100644
--- a/configure.in
+++ b/configure.in
@@ -607,40 +607,61 @@ AC_SUBST([SASL_LIBS])
 dnl PolicyKit library
 POLKIT_CFLAGS=
 POLKIT_LIBS=
+PKCHECK_PATH=
 AC_ARG_WITH([polkit],
   [  --with-polkit         use PolicyKit for UNIX socket access checks],
   [],
   [with_polkit=check])
 
+with_polkit0=no
+with_polkit1=no
 if test "x$with_polkit" = "xyes" -o "x$with_polkit" = "xcheck"; then
-  PKG_CHECK_MODULES(POLKIT, polkit-dbus >= $POLKIT_REQUIRED,
-    [with_polkit=yes], [
-    if test "x$with_polkit" = "xcheck" ; then
-       with_polkit=no
-    else
-       AC_MSG_ERROR(
-         [You must install PolicyKit >= $POLKIT_REQUIRED to compile libvirt])
-    fi
-  ])
-  if test "x$with_polkit" = "xyes" ; then
+  dnl Check for new polkit first - just a binary
+  AC_PATH_PROG([PKCHECK_PATH],[pkcheck], [], [/usr/sbin:$PATH])
+  if test "x$PKCHECK_PATH" != "x" ; then
+    AC_DEFINE_UNQUOTED([PKCHECK_PATH],["$PKCHECK_PATH"],[Location of pkcheck program])
     AC_DEFINE_UNQUOTED([HAVE_POLKIT], 1,
-      [use PolicyKit for UNIX socket access checks])
-
-    old_CFLAGS=$CFLAGS
-    old_LDFLAGS=$LDFLAGS
-    CFLAGS="$CFLAGS $POLKIT_CFLAGS"
-    LDFLAGS="$LDFLAGS $POLKIT_LIBS"
-    AC_CHECK_FUNCS([polkit_context_is_caller_authorized])
-    CFLAGS="$old_CFLAGS"
-    LDFLAGS="$old_LDFLAGS"
-
-    AC_PATH_PROG([POLKIT_AUTH], [polkit-auth])
-    if test "x$POLKIT_AUTH" != "x"; then
-      AC_DEFINE_UNQUOTED([POLKIT_AUTH],["$POLKIT_AUTH"],[Location of polkit-auth program])
+        [use PolicyKit for UNIX socket access checks])
+    AC_DEFINE_UNQUOTED([HAVE_POLKIT1], 1,
+        [use PolicyKit for UNIX socket access checks])
+    with_polkit="yes"
+    with_polkit1="yes"
+  else
+    dnl Check for old polkit second - library + binary
+    PKG_CHECK_MODULES(POLKIT, polkit-dbus >= $POLKIT_REQUIRED,
+      [with_polkit=yes], [
+      if test "x$with_polkit" = "xcheck" ; then
+         with_polkit=no
+      else
+         AC_MSG_ERROR(
+           [You must install PolicyKit >= $POLKIT_REQUIRED to compile libvirt])
+      fi
+    ])
+    if test "x$with_polkit" = "xyes" ; then
+      AC_DEFINE_UNQUOTED([HAVE_POLKIT], 1,
+        [use PolicyKit for UNIX socket access checks])
+      AC_DEFINE_UNQUOTED([HAVE_POLKIT0], 1,
+        [use PolicyKit for UNIX socket access checks])
+
+      old_CFLAGS=$CFLAGS
+      old_LDFLAGS=$LDFLAGS
+      CFLAGS="$CFLAGS $POLKIT_CFLAGS"
+      LDFLAGS="$LDFLAGS $POLKIT_LIBS"
+      AC_CHECK_FUNCS([polkit_context_is_caller_authorized])
+      CFLAGS="$old_CFLAGS"
+      LDFLAGS="$old_LDFLAGS"
+
+      AC_PATH_PROG([POLKIT_AUTH], [polkit-auth])
+      if test "x$POLKIT_AUTH" != "x"; then
+        AC_DEFINE_UNQUOTED([POLKIT_AUTH],["$POLKIT_AUTH"],[Location of polkit-auth program])
+      fi
+      with_polkit0="yes"
     fi
   fi
 fi
 AM_CONDITIONAL([HAVE_POLKIT], [test "x$with_polkit" = "xyes"])
+AM_CONDITIONAL([HAVE_POLKIT0], [test "x$with_polkit0" = "xyes"])
+AM_CONDITIONAL([HAVE_POLKIT1], [test "x$with_polkit1" = "xyes"])
 AC_SUBST([POLKIT_CFLAGS])
 AC_SUBST([POLKIT_LIBS])
 
@@ -1621,7 +1642,11 @@ else
 AC_MSG_NOTICE([   avahi: no])
 fi
 if test "$with_polkit" = "yes" ; then
-AC_MSG_NOTICE([  polkit: $POLKIT_CFLAGS $POLKIT_LIBS])
+if test "$with_polkit0" = "yes" ; then
+AC_MSG_NOTICE([  polkit: $POLKIT_CFLAGS $POLKIT_LIBS (version 0)])
+else
+AC_MSG_NOTICE([  polkit: $PKCHECK_PATH (version 1)])
+fi
 else
 AC_MSG_NOTICE([  polkit: no])
 fi
diff --git a/qemud/Makefile.am b/qemud/Makefile.am
index 959ff88..3d143da 100644
--- a/qemud/Makefile.am
+++ b/qemud/Makefile.am
@@ -21,7 +21,8 @@ EXTRA_DIST =						\
 	remote_protocol.x				\
 	libvirtd.conf					\
 	libvirtd.init.in				\
-	libvirtd.policy					\
+	libvirtd.policy-0				\
+	libvirtd.policy-1				\
 	libvirtd.sasl					\
 	libvirtd.sysconf				\
 	libvirtd.aug                                    \
@@ -147,7 +148,13 @@ endif
 libvirtd_LDADD += ../src/libvirt.la
 
 if HAVE_POLKIT
+if HAVE_POLKIT0
 policydir = $(datadir)/PolicyKit/policy
+policyfile = libvirtd.policy-0
+else
+policydir = $(datadir)/polkit-1/actions
+policyfile = libvirtd.policy-1
+endif
 endif
 
 if HAVE_AVAHI
@@ -197,7 +204,7 @@ endif
 if HAVE_POLKIT
 install-data-polkit:: install-init
 	mkdir -p $(DESTDIR)$(policydir)
-	$(INSTALL_DATA) $(srcdir)/libvirtd.policy $(DESTDIR)$(policydir)/org.libvirt.unix.policy
+	$(INSTALL_DATA) $(srcdir)/$(policyfile) $(DESTDIR)$(policydir)/org.libvirt.unix.policy
 uninstall-data-polkit:: install-init
 	rm -f $(DESTDIR)$(policydir)/org.libvirt.unix.policy
 else
diff --git a/qemud/libvirtd.policy b/qemud/libvirtd.policy
deleted file mode 100644
index b6da946..0000000
--- a/qemud/libvirtd.policy
+++ /dev/null
@@ -1,42 +0,0 @@
-<!DOCTYPE policyconfig PUBLIC
- "-//freedesktop//DTD PolicyKit Policy Configuration 1.0//EN"
- "http://www.freedesktop.org/standards/PolicyKit/1.0/policyconfig.dtd";>
-
-<!--
-Policy definitions for libvirt daemon
-
-Copyright (c) 2007 Daniel P. Berrange <berrange redhat com>
-
-libvirt is licensed to you under the GNU Lesser General Public License
-version 2. See COPYING for details.
-
-NOTE: If you make changes to this file, make sure to validate the file
-using the polkit-policy-file-validate(1) tool. Changes made to this
-file are instantly applied.
--->
-
-<policyconfig>
-    <action id="org.libvirt.unix.monitor">
-      <description>Monitor local virtualized systems</description>
-      <message>System policy prevents monitoring of local virtualized systems</message>
-      <defaults>
-        <!-- Any program can use libvirt in read-only mode for monitoring,
-             even if not part of a session -->
-        <allow_any>yes</allow_any>
-        <allow_inactive>yes</allow_inactive>
-        <allow_active>yes</allow_active>
-      </defaults>
-    </action>
-
-    <action id="org.libvirt.unix.manage">
-      <description>Manage local virtualized systems</description>
-      <message>System policy prevents management of local virtualized systems</message>
-      <defaults>
-        <!-- Only a program in the active host session can use libvirt in
-             read-write mode for management, and we require user password -->
-        <allow_any>no</allow_any>
-        <allow_inactive>no</allow_inactive>
-        <allow_active>auth_admin_keep_session</allow_active>
-      </defaults>
-    </action>
-</policyconfig>
diff --git a/qemud/libvirtd.policy-0 b/qemud/libvirtd.policy-0
new file mode 100644
index 0000000..b6da946
--- /dev/null
+++ b/qemud/libvirtd.policy-0
@@ -0,0 +1,42 @@
+<!DOCTYPE policyconfig PUBLIC
+ "-//freedesktop//DTD PolicyKit Policy Configuration 1.0//EN"
+ "http://www.freedesktop.org/standards/PolicyKit/1.0/policyconfig.dtd";>
+
+<!--
+Policy definitions for libvirt daemon
+
+Copyright (c) 2007 Daniel P. Berrange <berrange redhat com>
+
+libvirt is licensed to you under the GNU Lesser General Public License
+version 2. See COPYING for details.
+
+NOTE: If you make changes to this file, make sure to validate the file
+using the polkit-policy-file-validate(1) tool. Changes made to this
+file are instantly applied.
+-->
+
+<policyconfig>
+    <action id="org.libvirt.unix.monitor">
+      <description>Monitor local virtualized systems</description>
+      <message>System policy prevents monitoring of local virtualized systems</message>
+      <defaults>
+        <!-- Any program can use libvirt in read-only mode for monitoring,
+             even if not part of a session -->
+        <allow_any>yes</allow_any>
+        <allow_inactive>yes</allow_inactive>
+        <allow_active>yes</allow_active>
+      </defaults>
+    </action>
+
+    <action id="org.libvirt.unix.manage">
+      <description>Manage local virtualized systems</description>
+      <message>System policy prevents management of local virtualized systems</message>
+      <defaults>
+        <!-- Only a program in the active host session can use libvirt in
+             read-write mode for management, and we require user password -->
+        <allow_any>no</allow_any>
+        <allow_inactive>no</allow_inactive>
+        <allow_active>auth_admin_keep_session</allow_active>
+      </defaults>
+    </action>
+</policyconfig>
diff --git a/qemud/libvirtd.policy-1 b/qemud/libvirtd.policy-1
new file mode 100644
index 0000000..6fa3a5e
--- /dev/null
+++ b/qemud/libvirtd.policy-1
@@ -0,0 +1,42 @@
+<!DOCTYPE policyconfig PUBLIC
+ "-//freedesktop//DTD PolicyKit Policy Configuration 1.0//EN"
+ "http://www.freedesktop.org/standards/PolicyKit/1.0/policyconfig.dtd";>
+
+<!--
+Policy definitions for libvirt daemon
+
+Copyright (c) 2007 Daniel P. Berrange <berrange redhat com>
+
+libvirt is licensed to you under the GNU Lesser General Public License
+version 2. See COPYING for details.
+
+NOTE: If you make changes to this file, make sure to validate the file
+using the polkit-policy-file-validate(1) tool. Changes made to this
+file are instantly applied.
+-->
+
+<policyconfig>
+    <action id="org.libvirt.unix.monitor">
+      <description>Monitor local virtualized systems</description>
+      <message>System policy prevents monitoring of local virtualized systems</message>
+      <defaults>
+        <!-- Any program can use libvirt in read-only mode for monitoring,
+             even if not part of a session -->
+        <allow_any>yes</allow_any>
+        <allow_inactive>yes</allow_inactive>
+        <allow_active>yes</allow_active>
+      </defaults>
+    </action>
+
+    <action id="org.libvirt.unix.manage">
+      <description>Manage local virtualized systems</description>
+      <message>System policy prevents management of local virtualized systems</message>
+      <defaults>
+        <!-- Only a program in the active host session can use libvirt in
+             read-write mode for management, and we require user password -->
+        <allow_any>no</allow_any>
+        <allow_inactive>no</allow_inactive>
+        <allow_active>auth_admin_keep</allow_active>
+      </defaults>
+    </action>
+</policyconfig>
diff --git a/qemud/qemud.c b/qemud/qemud.c
index 3e551ca..50b0cdd 100644
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -895,7 +895,7 @@ static struct qemud_server *qemudNetworkInit(struct qemud_server *server) {
     }
 #endif
 
-#ifdef HAVE_POLKIT
+#if HAVE_POLKIT0
     if (auth_unix_rw == REMOTE_AUTH_POLKIT ||
         auth_unix_ro == REMOTE_AUTH_POLKIT) {
         DBusError derr;
@@ -982,7 +982,7 @@ static struct qemud_server *qemudNetworkInit(struct qemud_server *server) {
             sock = sock->next;
         }
 
-#ifdef HAVE_POLKIT
+#if HAVE_POLKIT0
         if (server->sysbus)
             dbus_connection_unref(server->sysbus);
 #endif
diff --git a/qemud/qemud.h b/qemud/qemud.h
index 254db44..e8ce209 100644
--- a/qemud/qemud.h
+++ b/qemud/qemud.h
@@ -34,7 +34,7 @@
 #include <sasl/sasl.h>
 #endif
 
-#ifdef HAVE_POLKIT
+#if HAVE_POLKIT0
 #include <dbus/dbus.h>
 #endif
 
@@ -253,7 +253,7 @@ struct qemud_server {
 #if HAVE_SASL
     char **saslUsernameWhitelist;
 #endif
-#if HAVE_POLKIT
+#if HAVE_POLKIT0
     DBusConnection *sysbus;
 #endif
 };
diff --git a/qemud/remote.c b/qemud/remote.c
index d32d513..490a807 100644
--- a/qemud/remote.c
+++ b/qemud/remote.c
@@ -43,7 +43,7 @@
 #include <fnmatch.h>
 #include "virterror_internal.h"
 
-#ifdef HAVE_POLKIT
+#if HAVE_POLKIT0
 #include <polkit/polkit.h>
 #include <polkit-dbus/polkit-dbus.h>
 #endif
@@ -3106,7 +3106,80 @@ remoteDispatchAuthSaslStep (struct qemud_server *server ATTRIBUTE_UNUSED,
 #endif /* HAVE_SASL */
 
 
-#if HAVE_POLKIT
+#if HAVE_POLKIT1
+static int
+remoteDispatchAuthPolkit (struct qemud_server *server,
+                          struct qemud_client *client,
+                          virConnectPtr conn ATTRIBUTE_UNUSED,
+                          remote_error *rerr,
+                          void *args ATTRIBUTE_UNUSED,
+                          remote_auth_polkit_ret *ret)
+{
+    pid_t callerPid;
+    uid_t callerUid;
+    const char *action;
+    int status = -1;
+    char pidbuf[50];
+    int rv;
+
+    virMutexLock(&server->lock);
+    virMutexLock(&client->lock);
+    virMutexUnlock(&server->lock);
+
+    action = client->readonly ?
+        "org.libvirt.unix.monitor" :
+        "org.libvirt.unix.manage";
+
+    const char * const pkcheck [] = {
+      PKCHECK_PATH,
+      "--action-id", action,
+      "--process", pidbuf,
+      "--allow-user-interaction",
+      NULL
+    };
+
+    REMOTE_DEBUG("Start PolicyKit auth %d", client->fd);
+    if (client->auth != REMOTE_AUTH_POLKIT) {
+        VIR_ERROR0(_("client tried invalid PolicyKit init request"));
+        goto authfail;
+    }
+
+    if (qemudGetSocketIdentity(client->fd, &callerUid, &callerPid) < 0) {
+        VIR_ERROR0(_("cannot get peer socket identity"));
+        goto authfail;
+    }
+
+    VIR_INFO(_("Checking PID %d running as %d"), callerPid, callerUid);
+
+    rv = snprintf(pidbuf, sizeof pidbuf, "%d", callerPid);
+    if (rv < 0 || rv >= sizeof pidbuf) {
+        VIR_ERROR(_("Caller PID was too large %d"), callerPid);
+	goto authfail;
+    }
+
+    if (virRun(NULL, pkcheck, &status) < 0) {
+        VIR_ERROR(_("Cannot invoke %s"), PKCHECK_PATH);
+	goto authfail;
+    }
+    if (status != 0) {
+        VIR_ERROR(_("Policy kit denied action %s from pid %d, uid %d, result: %d\n"),
+                  action, callerPid, callerUid, status);
+        goto authfail;
+    }
+    VIR_INFO(_("Policy allowed action %s from pid %d, uid %d"),
+             action, callerPid, callerUid);
+    ret->complete = 1;
+    client->auth = REMOTE_AUTH_NONE;
+
+    virMutexUnlock(&client->lock);
+    return 0;
+
+authfail:
+    remoteDispatchAuthError(rerr);
+    virMutexUnlock(&client->lock);
+    return -1;
+}
+#elif HAVE_POLKIT0
 static int
 remoteDispatchAuthPolkit (struct qemud_server *server,
                           struct qemud_client *client,
@@ -3217,7 +3290,7 @@ authfail:
     return -1;
 }
 
-#else /* HAVE_POLKIT */
+#else /* !HAVE_POLKIT0 & !HAVE_POLKIT1*/
 
 static int
 remoteDispatchAuthPolkit (struct qemud_server *server ATTRIBUTE_UNUSED,
@@ -3231,7 +3304,7 @@ remoteDispatchAuthPolkit (struct qemud_server *server ATTRIBUTE_UNUSED,
     remoteDispatchAuthError(rerr);
     return -1;
 }
-#endif /* HAVE_POLKIT */
+#endif /* HAVE_POLKIT1 */
 
 
 /***************************************************************


-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]