This patch is a draft implementation to pass a domain
XML to a polkit access driver. With this new feature, a polkit rule can
verify the domain XML to be deployed on a host, and thus protect deploying a
malicious VM.

There is a discussion about this new feature in the following issue.

https://gitlab.com/libvirt/libvirt/-/issues/719

Any question, comment, and suggestion are welcome. Thanks,

Signed-off-by: Takuya Nakaike <nakaike@jp.ibm.com>
---
 src/access/viraccessdriverpolkit.c |  2 ++
 src/access/viraccessmanager.c      | 22 ++++++++++++++++++++++
 src/access/viraccessmanager.h      |  3 +++
 src/libvirt_private.syms           |  1 +
 src/qemu/qemu_driver.c             |  3 +++
 5 files changed, 31 insertions(+)

diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c
index 83381183a5..56457010e0 100644
--- a/src/access/viraccessdriverpolkit.c
+++ b/src/access/viraccessdriverpolkit.c
@@ -177,10 +177,12 @@ virAccessDriverPolkitCheckDomain(virAccessManager *manager,
                                  virAccessPermDomain perm)
 {
     char uuidstr[VIR_UUID_STRING_BUFLEN];
+    char *xml = virAccessManagerGetXMLDesc(domain);
     const char *attrs[] = {
         "connect_driver", driverName,
         "domain_name", domain->name,
         "domain_uuid", uuidstr,
+        "xml", xml != NULL ? xml : "",
         NULL,
     };
     virUUIDFormat(domain->uuid, uuidstr);
diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c
index 6d9fdee5f1..7be31eb19a 100644
--- a/src/access/viraccessmanager.c
+++ b/src/access/viraccessmanager.c
@@ -44,11 +44,13 @@ struct _virAccessManager {
     virObjectLockable parent;
 
     virAccessDriver *drv;
+    virDomainXMLOption *xmlopt;
     void *privateData;
 };
 
 static virClass *virAccessManagerClass;
 static virAccessManager *virAccessManagerDefault;
+static virDomainXMLOption *virAccessManagerXMLOption = NULL;
 
 static void virAccessManagerDispose(void *obj);
 
@@ -376,3 +378,23 @@ int virAccessManagerCheckStorageVol(virAccessManager *manager,
 
     return virAccessManagerSanitizeError(ret, driverName);
 }
+
+virDomainXMLOption *virAccessManagerGetXMLOption(void)
+{
+    return virAccessManagerXMLOption;
+}
+
+void virAccessManagerSetXMLOption(virDomainXMLOption *xmlopt)
+{
+    virObjectUnref(virAccessManagerXMLOption);
+
+    virAccessManagerXMLOption = virObjectRef(xmlopt);
+}
+
+char *virAccessManagerGetXMLDesc(virDomainDef *domain)
+{
+    if (virAccessManagerGetXMLOption() != NULL) {
+	return virDomainDefFormat(domain, virAccessManagerGetXMLOption(), 0);
+    }
+    return NULL;
+}
diff --git a/src/access/viraccessmanager.h b/src/access/viraccessmanager.h
index 2050ac9b85..3516f4f8e4 100644
--- a/src/access/viraccessmanager.h
+++ b/src/access/viraccessmanager.h
@@ -94,3 +94,6 @@ int virAccessManagerCheckStorageVol(virAccessManager *manager,
                                     virStoragePoolDef *pool,
                                     virStorageVolDef *vol,
                                     virAccessPermStorageVol perm);
+virDomainXMLOption *virAccessManagerGetXMLOption(void);
+void virAccessManagerSetXMLOption(virDomainXMLOption *xmlopt);
+char *virAccessManagerGetXMLDesc(virDomainDef *domain);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 33b93cbd3e..8bcbc9c478 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -19,6 +19,7 @@ virAccessManagerGetDefault;
 virAccessManagerNew;
 virAccessManagerNewStack;
 virAccessManagerSetDefault;
+virAccessManagerSetXMLOption;
 
 
 # access/viraccessperm.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d2eddbd9ae..f9880f8ee3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -105,6 +105,7 @@
 #include "virdomaincheckpointobjlist.h"
 #include "virutil.h"
 #include "backup_conf.h"
+#include "viraccessmanager.h"
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
@@ -835,6 +836,8 @@ qemuStateInitialize(bool privileged,
                                                            defsecmodel)))
         goto error;
 
+    virAccessManagerSetXMLOption(qemu_driver->xmlopt);
+
     qemu_driver->nbdkitCapsCache = qemuNbdkitCapsCacheNew(cfg->cacheDir);
 
     /* If hugetlbfs is present, then we need to create a sub-directory within
-- 
2.43.5

On Thu, Jan 09, 2025 at 12:03:58AM -0500, Takuya Nakaike wrote:
> This patch is a draft implementation to pass a domain
> XML to a polkit access driver. With this new feature, a polkit rule can
> verify the domain XML to be deployed on a host, and thus protect deploying a
> malicious VM.
> 
> There is a discussion about this new feature in the following issue.
> 
> https://gitlab.com/libvirt/libvirt/-/issues/719
> 
> Any question, comment, and suggestion are welcome. Thanks,

Lets keep discussion on that issue. My comments there express why I think
this is a bad approach that should not be merged.

> diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c
> index 83381183a5..56457010e0 100644
> --- a/src/access/viraccessdriverpolkit.c
> +++ b/src/access/viraccessdriverpolkit.c
> @@ -177,10 +177,12 @@ virAccessDriverPolkitCheckDomain(virAccessManager *manager,
>                                   virAccessPermDomain perm)
>  {
>      char uuidstr[VIR_UUID_STRING_BUFLEN];
> +    char *xml = virAccessManagerGetXMLDesc(domain);
>      const char *attrs[] = {
>          "connect_driver", driverName,
>          "domain_name", domain->name,
>          "domain_uuid", uuidstr,
> +        "xml", xml != NULL ? xml : "",
>          NULL,
>      };

NB, that's a memory leak


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|