From 85750b0466aa3719d3d2447abaab2e87db92f552 Mon Sep 17 00:00:00 2001

Message-Id: <85750b0466aa3719d3d2447abaab2e87db92f552@dist-git>

From: John Ferlan <jferlan@redhat.com>

Date: Mon, 5 Nov 2018 07:48:37 -0500

Subject: [PATCH] access: Modify the VIR_ERR_ACCESS_DENIED to include

 driverName

https://bugzilla.redhat.com/show_bug.cgi?id=1631608 (RHEL 8.0)

https://bugzilla.redhat.com/show_bug.cgi?id=1631606 (RHEL 7.7)

Changes made to manage and utilize a secondary connection

driver to APIs outside the scope of the primary connection

driver have resulted in some confusion processing polkit rules

since the simple "access denied" error message doesn't provide

enough of a clue when combined with the "authentication failed:

access denied by policy" as to which connection driver refused

or failed the ACL check.

In order to provide some context, let's modify the existing

"access denied" error returne from the various vir*EnsureACL

API's to provide the connection driver name that is causing

the failure. This should provide the context for writing the

polkit rules that would allow access via the driver.

Signed-off-by: John Ferlan <jferlan@redhat.com>

ACKed-by: Michal Privoznik <mprivozn@redhat.com>

(cherry picked from commit ccc72d5cbdd85f66cb737134b3be40aac1df03ef)

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/access/viraccessmanager.c | 25 +++++++++++++------------

 src/rpc/gendispatch.pl        |  2 +-

 src/util/virerror.c           |  4 ++--

 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c

index e7b5bf38da..1dfff32b9d 100644

--- a/src/access/viraccessmanager.c

+++ b/src/access/viraccessmanager.c

@@ -196,11 +196,12 @@ static void virAccessManagerDispose(void *object)

  * should the admin need to debug things

  */

 static int

-virAccessManagerSanitizeError(int ret)

+virAccessManagerSanitizeError(int ret,

+                              const char *driverName)

 {

     if (ret < 0) {

         virResetLastError();

-        virAccessError(VIR_ERR_ACCESS_DENIED, NULL);

+        virAccessError(VIR_ERR_ACCESS_DENIED, driverName, NULL);

     }

     return ret;

@@ -217,7 +218,7 @@ int virAccessManagerCheckConnect(virAccessManagerPtr manager,

     if (manager->drv->checkConnect)

         ret = manager->drv->checkConnect(manager, driverName, perm);

-    return virAccessManagerSanitizeError(ret);

+    return virAccessManagerSanitizeError(ret, driverName);

 }

@@ -233,7 +234,7 @@ int virAccessManagerCheckDomain(virAccessManagerPtr manager,

     if (manager->drv->checkDomain)

         ret = manager->drv->checkDomain(manager, driverName, domain, perm);

-    return virAccessManagerSanitizeError(ret);

+    return virAccessManagerSanitizeError(ret, driverName);

 }

 int virAccessManagerCheckInterface(virAccessManagerPtr manager,

@@ -248,7 +249,7 @@ int virAccessManagerCheckInterface(virAccessManagerPtr manager,

     if (manager->drv->checkInterface)

         ret = manager->drv->checkInterface(manager, driverName, iface, perm);

-    return virAccessManagerSanitizeError(ret);

+    return virAccessManagerSanitizeError(ret, driverName);

 }

 int virAccessManagerCheckNetwork(virAccessManagerPtr manager,

@@ -263,7 +264,7 @@ int virAccessManagerCheckNetwork(virAccessManagerPtr manager,

     if (manager->drv->checkNetwork)

         ret = manager->drv->checkNetwork(manager, driverName, network, perm);

-    return virAccessManagerSanitizeError(ret);

+    return virAccessManagerSanitizeError(ret, driverName);

 }

 int virAccessManagerCheckNodeDevice(virAccessManagerPtr manager,

@@ -278,7 +279,7 @@ int virAccessManagerCheckNodeDevice(virAccessManagerPtr manager,

     if (manager->drv->checkNodeDevice)

         ret = manager->drv->checkNodeDevice(manager, driverName, nodedev, perm);

-    return virAccessManagerSanitizeError(ret);

+    return virAccessManagerSanitizeError(ret, driverName);

 }

 int virAccessManagerCheckNWFilter(virAccessManagerPtr manager,

@@ -293,7 +294,7 @@ int virAccessManagerCheckNWFilter(virAccessManagerPtr manager,

     if (manager->drv->checkNWFilter)

         ret = manager->drv->checkNWFilter(manager, driverName, nwfilter, perm);

-    return virAccessManagerSanitizeError(ret);

+    return virAccessManagerSanitizeError(ret, driverName);

 }

 int virAccessManagerCheckNWFilterBinding(virAccessManagerPtr manager,

@@ -308,7 +309,7 @@ int virAccessManagerCheckNWFilterBinding(virAccessManagerPtr manager,

     if (manager->drv->checkNWFilterBinding)

         ret = manager->drv->checkNWFilterBinding(manager, driverName, binding, perm);

-    return virAccessManagerSanitizeError(ret);

+    return virAccessManagerSanitizeError(ret, driverName);

 }

 int virAccessManagerCheckSecret(virAccessManagerPtr manager,

@@ -323,7 +324,7 @@ int virAccessManagerCheckSecret(virAccessManagerPtr manager,

     if (manager->drv->checkSecret)

         ret = manager->drv->checkSecret(manager, driverName, secret, perm);

-    return virAccessManagerSanitizeError(ret);

+    return virAccessManagerSanitizeError(ret, driverName);

 }

 int virAccessManagerCheckStoragePool(virAccessManagerPtr manager,

@@ -338,7 +339,7 @@ int virAccessManagerCheckStoragePool(virAccessManagerPtr manager,

     if (manager->drv->checkStoragePool)

         ret = manager->drv->checkStoragePool(manager, driverName, pool, perm);

-    return virAccessManagerSanitizeError(ret);

+    return virAccessManagerSanitizeError(ret, driverName);

 }

 int virAccessManagerCheckStorageVol(virAccessManagerPtr manager,

@@ -354,5 +355,5 @@ int virAccessManagerCheckStorageVol(virAccessManagerPtr manager,

     if (manager->drv->checkStorageVol)

         ret = manager->drv->checkStorageVol(manager, driverName, pool, vol, perm);

-    return virAccessManagerSanitizeError(ret);

+    return virAccessManagerSanitizeError(ret, driverName);

 }

diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl

index 0c4648c0fb..f599002056 100755

--- a/src/rpc/gendispatch.pl

+++ b/src/rpc/gendispatch.pl

@@ -2199,7 +2199,7 @@ elsif ($mode eq "client") {

                     print "        virObjectUnref(mgr);\n";

                     if ($action eq "Ensure") {

                         print "        if (rv == 0)\n";

-                        print "            virReportError(VIR_ERR_ACCESS_DENIED, NULL);\n";

+                        print "            virReportError(VIR_ERR_ACCESS_DENIED, conn->driver->name, NULL);\n";

                         print "        return $fail;\n";

                     } else {

                         print "        virResetLastError();\n";

diff --git a/src/util/virerror.c b/src/util/virerror.c

index f198f27957..5f50fa0349 100644

--- a/src/util/virerror.c

+++ b/src/util/virerror.c

@@ -1439,9 +1439,9 @@ virErrorMsg(virErrorNumber error, const char *info)

             break;

         case VIR_ERR_ACCESS_DENIED:

             if (info == NULL)

-                errmsg = _("access denied");

+                errmsg = _("access denied from '%s'");

             else

-                errmsg = _("access denied: %s");

+                errmsg = _("access denied from '%s': %s");

             break;

         case VIR_ERR_DBUS_SERVICE:

             if (info == NULL)

-- 

2.19.1