[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 <mprivozn at redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange at 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;
>   }
>   
>   
>