PolicyKit: Check auth before asking client to obtain it

I previously mentioned [1] a PolicyKit issue where libvirt would
proceed with authentication even though polkit-auth failed:

testusr xen134:~> virsh list --all
Attempting to obtain authorization for org.libvirt.unix.manage.
polkit-grant-helper: given auth type (8 -> yes) is bogus
Failed to obtain authorization for org.libvirt.unix.manage.
 Id Name                 State
----------------------------------
  0 Domain-0             running
  - sles11sp1-pv         shut off

AFAICT, libvirt attempts to obtain a privilege it already has,
causing polkit-auth to fail with above message.  Instead of calling
obtain and then checking auth, IMO the workflow should be for the
server to check auth first, and if that fails ask the client to
obtain it and check again.  This workflow also allows for checking
only successful exit of polkit-auth in virConnectAuthGainPolkit().

[1] https://www.redhat.com/archives/libvir-list/2011-December/msg00837.html

diff --git a/src/libvirt.c b/src/libvirt.c
index c288f000caaa67d3f58cfb3ff95d5bbccee9c895..c7a23450161211733abfe887f8d5cb804a6b0170 100644 (file)
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -119,7 +119,7 @@ static int virConnectAuthGainPolkit(const char *privilege) {
 
     cmd = virCommandNewArgList(POLKIT_AUTH, "--obtain", privilege, NULL);
     if (virCommandRun(cmd, &status) < 0 ||
-        status > 1)
+        status > 0)
         goto cleanup;
 
     ret = 0;

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 75804771ee369f180545fccba053c8e6b94b8f24..e28840b9816dcfb02f56cd32d29974c23710614b 100644 (file)
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -3121,6 +3121,14 @@ remoteAuthPolkit (virConnectPtr conn, struct private_data *priv,
     };
     VIR_DEBUG("Client initialize PolicyKit-0 authentication");
 
+    /* Check auth first and if it succeeds we are done. */
+    memset (&ret, 0, sizeof ret);
+    if (call (conn, priv, 0, REMOTE_PROC_AUTH_POLKIT,
+              (xdrproc_t) xdr_void, (char *)NULL,
+              (xdrproc_t) xdr_remote_auth_polkit_ret, (char *) &ret) == 0)
+        goto out;
+
+    /* Auth failed.  Ask client to obtain it and check again. */
     if (auth && auth->cb) {
         /* Check if the necessary credential type for PolicyKit is supported */
         for (i = 0 ; i < auth->ncredtype ; i++) {
@@ -3138,9 +3146,11 @@ remoteAuthPolkit (virConnectPtr conn, struct private_data *priv,
             }
         } else {
             VIR_DEBUG("Client auth callback does not support PolicyKit");
+            return -1;
         }
     } else {
         VIR_DEBUG("No auth callback provided");
+        return -1;
     }
 
     memset (&ret, 0, sizeof ret);
@@ -3150,6 +3160,7 @@ remoteAuthPolkit (virConnectPtr conn, struct private_data *priv,
         return -1; /* virError already set by call */
     }
 
+out:
     VIR_DEBUG("PolicyKit-0 authentication complete");
     return 0;
 }