Re: [Libvir] libvirt daemon UNIX socket auth with PolicyKit

On Wed, Aug 08, 2007 at 05:22:33AM +0100, Daniel P. Berrange wrote: > In summary what I really need for virt-manager is > > - Always run as non-root > - Authenticate for local guest management (ie read+write) Okay, problem understood. > - libvirtd defines two actions it can check called 'libvirt-local-monitor > (read only monitoring of state), and 'libvirt-local-manage' (full > read-write management). Would the model allow things like being able to manage domains run by the user, but only monitor domains from someone else. I understand it may not always be possible to store that information, but if available a finer grained control based on context would make sense. Things like PolicyKit are being added among other things for better user switching mechanism, and the domains run by an user are in my opinion an extension of the user session, and having per user control would make sense IMHO.

> - Both libvirt-sock and libvirt-sock-ro are mode 0777 (ie all users) > > - A connection arrives on the UNIX domain socket either libvirt-sock-ro > or libvirt-sock > > - libvirtd use SO_PEERCRED to get the PID of the client I'm just wondering a bit about the portability, I remember that for gamin we had to use at least 2 different authentication mechanism to assert reliably the identity of the connecting process. Might be worth looking again at D-Bus code for this.

> +enum libvirtd_auth { > + LIBVIRTD_AUTH_NONE, > + LIBVIRTD_AUTH_TLS, > +#ifdef HAVE_POLICY_KIT > + LIBVIRTD_AUTH_POLICYKIT, > +#endif > +}; Hum, I would keep the enum present even if it's not detected, the value won't be used but it should still be defined IMHO.

> void qemudLog(int priority, const char *fmt, ...) > Index: qemud/libvirtd.policy > =================================================================== > RCS file: qemud/libvirtd.policy > diff -N qemud/libvirtd.policy > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ qemud/libvirtd.policy 8 Aug 2007 04:10:45 -0000 > @@ -0,0 +1,41 @@ > +<!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(a)redhat.com&gt; > + > +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> > + <group id="libvirtd"> > + <description>Virtualization</description> > + > + <policy id="libvirtd-local-monitor"> > + <description>Monitor local virtualized systems</description> > + <message>System policy prevents monitoring local virtualized systems</message> > + <defaults> > + <allow_inactive>yes</allow_inactive> > + <allow_active>yes</allow_active> > + </defaults> > + </policy> > + > + <policy id="libvirtd-local-manage"> > + <description>Manage local virtualized systems</description> > + <message>System policy prevents managing local virtualized systems</message> > + <defaults> > + <allow_inactive>no</allow_inactive> > + <allow_active>auth_admin</allow_active> > + </defaults> > + </policy> > + > + </group> > +</policyconfig> Cool, more XML, we should actually check that PolicyKit install the http://www.freedesktop.org/standards/PolicyKit/1.0/policyconfig.dtd DTD in the XML catalog. I need to install F-8 test1 somewhere !

> - if (!client->tls) { > + switch (client->auth) { > +#ifdef HAVE_POLICY_KIT > + case LIBVIRTD_AUTH_POLICYKIT: I would keep the case out, add an error if HAVE_POLICY_KIT is not #defined which should not happen but...

> + { > + PolKitCaller *pkcaller = NULL; > + PolKitAction *pkaction = NULL; > + PolKitContext *pkcontext = NULL; > + PolKitError *pkerr; > + PolKitResult pkresult; > + const char *action = sock->readonly ? > + "libvirtd-local-monitor" : > + "libvirtd-local-manage"; > + pid_t callerPid; > + DBusError err; > +#ifdef SO_PEERCRED > + struct ucred cr; > + unsigned int cr_len = sizeof (cr); > + > + if (getsockopt (fd, SOL_SOCKET, SO_PEERCRED, &cr, &cr_len) < 0) { > + qemudLog(QEMUD_ERR, "Failed to verify client credentials: %s", strerror(errno)); > + close(fd); > + free(client); > + return -1; > + } > + callerPid = cr.pid; > +#else > + /* XXX Many more OS support UNIX socket credentials we could port to ....*/ We should match with DBus code. If PolicyKit is defined, then that mean DBus is ported there and DBus do various socket creds checks last time I looked like CMSGCRED (but that's not as easy to use as SO_PEERCRED clearly).

> +#error "UNIX socket credentials not supported/implemeneted on this platform" > +#endif > + > + dbus_error_init(&err); > + if (!(pkcaller = polkit_caller_new_from_pid(server->sysbus, callerPid, &err))) { > + qemudLog(QEMUD_ERR, "Failed to lookup policy kit caller: %s", err.message); > + dbus_error_free(&err); > + close(fd); > + free(client); > + return -1; > + } > + > + if (!(pkaction = polkit_action_new())) { > + qemudLog(QEMUD_ERR, "Failed to create polkit action %s\n", strerror(errno)); > + polkit_caller_unref(pkcaller); > + close(fd); > + free(client); > + return -1; > + } > + polkit_action_set_action_id(pkaction, action); > + > + if (!(pkcontext = polkit_context_new()) || > + !polkit_context_init(pkcontext, &pkerr)) { > + qemudLog(QEMUD_ERR, "Failed to create polkit context %s\n", > + pkerr ? polkit_error_get_error_message(pkerr) : strerror(errno)); > + if (pkerr) > + polkit_error_free(pkerr); > + polkit_caller_unref(pkcaller); > + polkit_action_unref(pkaction); > + dbus_error_free(&err); > + close(fd); > + free(client); > + return -1; > + } > + > + pkresult = polkit_context_can_caller_do_action(pkcontext, pkaction, pkcaller); > + polkit_context_unref(pkcontext); > + polkit_caller_unref(pkcaller); > + polkit_action_unref(pkaction); > + if (pkresult != POLKIT_RESULT_YES) { > + qemudLog(QEMUD_ERR, "Policy kit denied action %s from pid %d, result: %s\n", > + action, callerPid, polkit_result_to_string_representation(pkresult)); > + close(fd); > + free(client); > + return -1; > + } > + } > + /* Allowed by policy kit, so fallthrough to generic setup... */ > +#endif The availability of the uid can be derived from the pid and even in the absence of PolicyKit I think we could embbed the same kind of policies as we do now and enforce them without relying on the filesystem attributes.

> > dnl Allow to build without Xen, QEMU/KVM, test or remote driver > AC_ARG_WITH(xen, > -[ --with-xen add XEN support (on)]) > +[ --with-xen add XEN support (on)],[],[with_xen=yes]) > AC_ARG_WITH(qemu, > -[ --with-qemu add QEMU/KVM support (on)]) > +[ --with-qemu add QEMU/KVM support (on)],[],[with_qemu=yes]) > AC_ARG_WITH(openvz, > -[ --with-openvz add OpenVZ support (off)]) > +[ --with-openvz add OpenVZ support (off)],[],[with_openvz=no]) > AC_ARG_WITH(test, > -[ --with-test add test driver support (on)]) > +[ --with-test add test driver support (on)],[],[with_test=yes]) > AC_ARG_WITH(remote, > -[ --with-remote add remote driver support (on)]) > +[ --with-remote add remote driver support (on)],[],[with_remote=yes]) > > dnl > dnl specific tests to setup DV devel environments with debug etc ... > @@ -95,7 +97,7 @@ AC_SUBST(STATIC_BINARIES) > dnl --enable-debug=(yes|no) > AC_ARG_ENABLE(debug, > AC_HELP_STRING([--enable-debug=no/yes], > - [enable debugging output])) > + [enable debugging output]),[],[enable_debug=no]) > if test x"$enable_debug" = x"yes"; then > AC_DEFINE(ENABLE_DEBUG, [], [whether debugging is enabled]) > fi [...] > + > +echo > +echo "Drivers:" > +echo " Xen: $with_xen" > +echo " QEMU: $with_qemu" > +echo " Test: $with_test" > +echo " OpenVZ: $with_openvz" > +echo " Remote: $with_remote" > +echo > +echo "General features:" > +echo " Debug: $enable_debug" > +if test "$with_remote" = "yes"; then > + echo > + echo "Remote driver features:" and that too :-)