[libvirt] [PATCH 3/3] virsh: Add support for text based polkit authentication

[Daniel P. Berrange]

On Wed, Feb 10, 2016 at 02:46:36PM -0500, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=872166
> 
> When the login session doesn't have an ssh -X type display agent in
> order for libvirtd to run the polkit session authentication, attempts
> to run 'virsh -c qemu:///system list' from an unauthorized user (or one
> that isn't part of the libvirt /etc/group) will fail with the following
> error from libvirtd:
> 
> error: authentication failed: no agent is available to authenticate -
>        polkit agent 'org.libvirt.unix.manage' is not available
> 
> In order to handle the local authentication, add a new switch "--pkauth"
> to virsh which will make use of the virPolkitAgent* API's in order to
> handle the text authentication when not a readonly request.
> 
> With this patch in place, the following occurs:
> 
> $ virsh --pkauth -c qemu:///system list
> ==== AUTHENTICATING FOR org.libvirt.unix.manage ===
> System policy prevents management of local virtualized systems
> Authenticating as: Some User (SUser)
> Password:
> ==== AUTHENTICATION COMPLETE ===
>  Id    Name                           State
>  ----------------------------------------------------
>   1     somedomain                     running
> 
> $
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  tools/virsh.c   | 22 +++++++++++++++++++---
>  tools/virsh.h   |  2 ++
>  tools/virsh.pod | 10 ++++++++++
>  tools/vsh.h     |  1 +
>  4 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index b96dbda..83cebe0 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -143,6 +143,7 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly)
>      int interval = 5; /* Default */
>      int count = 6;    /* Default */
>      bool keepalive_forced = false;
> +    virCommandPtr pkagent = NULL;
>  
>      if (ctl->keepalive_interval >= 0) {
>          interval = ctl->keepalive_interval;
> @@ -153,10 +154,17 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly)
>          keepalive_forced = true;
>      }
>  
> +    if (!readonly && ctl->pkauth) {
> +        if (!(pkagent = virPolkitAgentCreate()))
> +            return NULL;
> +        if (virPolkitAgentCheck() < 0)
> +            goto cleanup;

I'm not sure why you need to run pkcheck here - the auth check is
something the server side does - it doesn't make sense for the
client to be doing that.


> @@ -701,13 +713,14 @@ virshParseArgv(vshControl *ctl, int argc, char **argv)
>          {"readonly", no_argument, NULL, 'r'},
>          {"timing", no_argument, NULL, 't'},
>          {"version", optional_argument, NULL, 'v'},
> +        {"pkauth", no_argument, NULL, 'p'},

I think it would nice if we did without this extra argument and made things
"just work". The way polkit agents work is that they have to claim a well
known path on the system dbus instance. If we connect to DBus and check for
existance of the "/org/freedesktop/PolicyKit1/AuthenticationAgent" we will
know whether we should try to provide an auth agent or not.

That said I wonder if there's cause to support a scenario where polkit
agent is not running and people want to run virsh without the agent
still.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|