From 1d35f6ffe15f860914294f6a97de428d357a1fc9 Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Thu, 14 Jan 2016 14:34:28 -0500 Subject: [PATCH] polkit: Adjust message when authentication agent isn't found When there isn't a ssh -X type session running and a user has not been added to the libvirt group, attempts to run 'virsh -c qemu:///system' commands from an otherwise unprivileged user will fail with rather generic or opaque error message: "error: authentication failed: no agent is available to authenticate" This patch will adjust the error code and message to help reflect the situation that the problem is the requested mechanism is UNAVAILABLE and a slightly more descriptive error. The result on a failure then becomes: "error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage'" A bit more history on this - at one time a failure generated the following type message when running the 'pkcheck' as a subprocess: "error: authentication failed: polkit\56retains_authorization_after_challenge=1 Authorization requires authentication but no agent is available." but, a patch was generated to adjust the error message to help provide more details about what failed. This was pushed as commit id '96a108c99'. That patch prepended a "polkit: " to the output. It really didn't solve the problem, but gave a hint. After some time it was deemed using DBus API calls directly was a better way to go (since pkcheck calls them anyway). So, commit id '1b854c76' (more or less) copied the code from remoteDispatchAuthPolkit and adjusted it. Then commit id 'c7542573' adjusted the remote.c code to call the new API (virPolkitCheckAuth). Finally, commit id '308c0c5a' altered the code to call DBus APIs directly. In doing so, it reverted the failing error message to the generic message that would have been received from DBus anyway. --- include/libvirt/virterror.h | 3 ++- src/util/virerror.c | 8 +++++++- src/util/virpolkit.c | 8 +++++--- tests/virpolkittest.c | 7 ++++--- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index c6d1a761fb..83f76d84cb 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -4,7 +4,7 @@ * Description: Provides the interfaces of the libvirt library to handle * errors raised while using the library. * - * Copyright (C) 2006-2015 Red Hat, Inc. + * Copyright (C) 2006-2016 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 @@ -310,6 +310,7 @@ typedef enum { CPU*/ VIR_ERR_XML_INVALID_SCHEMA = 92, /* XML document doesn't validate against schema */ VIR_ERR_MIGRATE_FINISH_OK = 93, /* Finish API succeeded but it is expected to return NULL */ + VIR_ERR_AUTH_UNAVAILABLE = 94, /* authentication unavailable */ } virErrorNumber; /** diff --git a/src/util/virerror.c b/src/util/virerror.c index e1bcf52e07..377c2b11a2 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1,7 +1,7 @@ /* * virerror.c: error handling and reporting code for libvirt * - * Copyright (C) 2006, 2008-2015 Red Hat, Inc. + * Copyright (C) 2006, 2008-2016 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 @@ -1095,6 +1095,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("authentication cancelled: %s"); break; + case VIR_ERR_AUTH_UNAVAILABLE: + if (info == NULL) + errmsg = _("authentication unavailable"); + else + errmsg = _("authentication unavailable: %s"); + break; case VIR_ERR_NO_STORAGE_POOL: if (info == NULL) errmsg = _("Storage pool not found"); diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c index 8da91f2cdb..df707f1a2d 100644 --- a/src/util/virpolkit.c +++ b/src/util/virpolkit.c @@ -1,7 +1,7 @@ /* * virpolkit.c: helpers for using polkit APIs * - * Copyright (C) 2013, 2014 Red Hat, Inc. + * Copyright (C) 2013, 2014, 2016 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 @@ -121,8 +121,10 @@ int virPolkitCheckAuth(const char *actionid, virReportError(VIR_ERR_AUTH_CANCELLED, "%s", _("user cancelled authentication process")); else if (is_challenge) - virReportError(VIR_ERR_AUTH_FAILED, "%s", - _("no agent is available to authenticate")); + virReportError(VIR_ERR_AUTH_UNAVAILABLE, + _("no polkit agent available to authenticate " + "action '%s'"), + actionid); else virReportError(VIR_ERR_AUTH_FAILED, "%s", _("access denied by policy")); diff --git a/tests/virpolkittest.c b/tests/virpolkittest.c index 1ef7635fd8..73f001b6a1 100644 --- a/tests/virpolkittest.c +++ b/tests/virpolkittest.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2013, 2014 Red Hat, Inc. + * Copyright (C) 2013, 2014, 2016 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 @@ -220,8 +220,9 @@ static int testPolkitAuthChallenge(const void *opaque ATTRIBUTE_UNUSED) } err = virGetLastError(); - if (!err || !strstr(err->message, - _("no agent is available to authenticate"))) { + if (!err || err->domain != VIR_FROM_POLKIT || + err->code != VIR_ERR_AUTH_UNAVAILABLE || + !strstr(err->message, _("no polkit agent available to authenticate"))) { fprintf(stderr, "Incorrect error response\n"); goto cleanup; }