On Wed, Aug 08, 2007 at 05:35:09AM -0400, Daniel Veillard wrote:
> 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.
Its not clear whether PolicyKit is going to be suitable for fine-grained per-object
permissioning at this time. It is primarily concentrating on the coarse high level
access to system resources at this time. We'll just have to watch what direction it
evolves in. Personally I still think SELinux is probably the more likely long term
bet for fine grained MAC on this at this time. Perhaps PolicyKit will end up being
backed by/integrating with SELinux in the future.
> > - 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.
Portability sucks. There's at least 4/5 os specific impls - as per my other reply
to John I'd like to see this code isolated inside PolicyKit APIs themselves really.
> > +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.
I dunno - I preferred to keep it removed if not available so we don't accidentally
make use of it elsewhere in the code.
>
> > 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@xxxxxxxxxx>
> > +
> > +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 !
Install it in a Xen guest !
The RPM puts the DTD in /usr/share/PolicyKit/config.dtd
>
> > - 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...
Guess that would be safe enough.
>
> > + {
> > + 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).
Yep, I'd like to not have this code in libvirt at all really.
>
> > +#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.
Yes, in fact the 'struct ucred ' does have both a UID and PID field - I'm
only using the UID in this code, but if we wanted to use this as a check
even without PolicyKIt around we could do that.
> >
> > 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 :-)
Yes, I meant to separate this out for inclusion separately. In fact I
want to make it so that the configure script checks whether Xen, QEMU,
OpenVZ and GNUTLS are available and automatically enables the optional
set of drivers based on this. The --with-XXX flags would then merely
be to override the default - eg force a build without Xen even if its
available.
Regards,
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list