Re: [libvirt PATCH v3 08/10] src: set identity when opening secondary drivers

[John Ferlan]

On 5/12/21 9:33 AM, Daniel P. Berrangé wrote:

The drivers can all call virGetConnectXXX to open a connection to a
secondary driver. For example, when creating a encrypted storage volume,
the storage driver has to open a secret driver connection, or when
starting a guest, the QEMU driver has to open the network driver to
lookup a virtual network.

When using monolithic libvirtd, the connection has the same effective
identity as the client, since everything is still in the same process.
When using the modular daemons, however, the remote daemon sees the
identity of the calling daemon. This is a mistake as it results in
the modular daemons seeing the client with elevated privileges.

We need to pass on the current identity explicitly when opening the
secondary drivers. This is the same thing that is done by daemon RPC
dispatcher code when it is directly forwarding top level API calls
from virtproxyd and other daemons.

Reviewed-by: Michal Privoznik <mpriv...@redhat.com>
Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
---
  src/driver.c | 27 +++++++++++++++++++++++++++
  1 file changed, 27 insertions(+)

diff --git a/src/driver.c b/src/driver.c
index f8022d2522..227bb56e48 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -33,6 +33,8 @@
  #include "virstring.h"
  #include "virthread.h"
  #include "virutil.h"
+#include "viridentity.h"
+#include "datatypes.h"
  #include "configmake.h"

VIR_LOG_INIT("driver");

@@ -136,6 +138,7 @@ static virConnectPtr
  virGetConnectGeneric(virThreadLocal *threadPtr, const char *name)
  {
      virConnectPtr conn;
+    virErrorPtr saved;

if (virConnectCacheInitialize() < 0)

          return NULL;
@@ -153,8 +156,32 @@ virGetConnectGeneric(virThreadLocal *threadPtr, const char 
*name)

conn = virConnectOpen(uri);

          VIR_DEBUG("Opened new %s connection %p", name, conn);
+        if (!conn)
+            return NULL;
+
+        if (conn->driver->connectSetIdentity != NULL) {
+            g_autoptr(virIdentity) ident = NULL;
+            virTypedParameterPtr identparams = NULL;
+            int nidentparams = 0;
+
+            VIR_DEBUG("Attempting to delegate current identity");
+            if (!(ident = virIdentityGetCurrent()))
+                goto error;
+
+            if (virIdentityGetParameters(ident, &identparams, &nidentparams) < 
0)
+                goto error;
+
+            if (virConnectSetIdentity(conn, identparams, nidentparams, 0) < 0)
+                goto error;
+        }
      }
      return conn;
+
+ error:
+    saved = virSaveLastError();
+    virConnectClose(conn);
+    virSetError(saved);

Coverity complains about leak here

Need a virFreeError(saved);

John

+    return NULL;
  }