[PATCH V3] libxl: Reject VM config referencing nwfilters

Jim Fehlig via Devel posted 1 patch 8 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20241008214900.30187-1-jfehlig@suse.com
src/libxl/libxl_domain.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
[PATCH V3] libxl: Reject VM config referencing nwfilters
Posted by Jim Fehlig via Devel 8 months, 1 week ago
The Xen libxl driver does not support nwfilter. Introduce a
deviceValidateCallback function with a check for nwfilters, returning
VIR_ERR_CONFIG_UNSUPPORTED if any are found. Also fail to start any
existing VMs referencing nwfilters.

Drivers generally ignore unrecognized XML configuration, but ignoring
a user's request to filter VM network traffic can be viewed as a
security issue.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---

V2:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/UHG4TMYRRHTHQSYAVDOB46KHZ6QPIT4O/

Changes in V3:
When starting a VM, use virDomainDefValidate to validate the def instead
of duplicating the checks.

Note: With the change in V3, the validation is only done when starting
a new VM. Validation is skipped if restoring a managed saved VM.

 src/libxl/libxl_domain.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 0f129ec69c..029b12fa3b 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -356,12 +356,30 @@ libxlDomainDefValidate(const virDomainDef *def,
     return 0;
 }
 
+static int
+libxlDomainDeviceDefValidate(const virDomainDeviceDef *dev,
+                             const virDomainDef *def,
+                             void *opaque G_GNUC_UNUSED,
+                             void *parseOpaque G_GNUC_UNUSED)
+{
+    if (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->filter) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("filterref is not supported in %1$s"),
+                       virDomainVirtTypeToString(def->virtType));
+        return -1;
+    }
+
+    return 0;
+}
+
+
 virDomainDefParserConfig libxlDomainDefParserConfig = {
     .macPrefix = { 0x00, 0x16, 0x3e },
     .netPrefix = LIBXL_GENERATED_PREFIX_XEN,
     .devicesPostParseCallback = libxlDomainDeviceDefPostParse,
     .domainPostParseCallback = libxlDomainDefPostParse,
     .domainValidateCallback = libxlDomainDefValidate,
+    .deviceValidateCallback = libxlDomainDeviceDefValidate,
 
     .features = VIR_DOMAIN_DEF_FEATURE_USER_ALIAS |
                 VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT |
@@ -1460,6 +1478,10 @@ libxlDomainStartNew(libxlDriverPrivate *driver,
                      managed_save_path);
 
         vm->hasManagedSave = false;
+    } else {
+        /* Validate configuration if starting a new VM */
+        if (virDomainDefValidate(vm->def, 0, driver->xmlopt, NULL) < 0)
+            goto cleanup;
     }
 
     ret = libxlDomainStart(driver, vm, start_paused, restore_fd, restore_ver);
-- 
2.35.3
Re: [PATCH V3] libxl: Reject VM config referencing nwfilters
Posted by Martin Kletzander 8 months, 1 week ago
On Tue, Oct 08, 2024 at 03:49:00PM -0600, Jim Fehlig via Devel wrote:
>The Xen libxl driver does not support nwfilter. Introduce a
>deviceValidateCallback function with a check for nwfilters, returning
>VIR_ERR_CONFIG_UNSUPPORTED if any are found. Also fail to start any
>existing VMs referencing nwfilters.
>
>Drivers generally ignore unrecognized XML configuration, but ignoring
>a user's request to filter VM network traffic can be viewed as a
>security issue.
>
>Signed-off-by: Jim Fehlig <jfehlig@suse.com>

Makes sense to me

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

>---
>
>V2:
>https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/UHG4TMYRRHTHQSYAVDOB46KHZ6QPIT4O/
>
>Changes in V3:
>When starting a VM, use virDomainDefValidate to validate the def instead
>of duplicating the checks.
>
>Note: With the change in V3, the validation is only done when starting
>a new VM. Validation is skipped if restoring a managed saved VM.
>
> src/libxl/libxl_domain.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
>diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
>index 0f129ec69c..029b12fa3b 100644
>--- a/src/libxl/libxl_domain.c
>+++ b/src/libxl/libxl_domain.c
>@@ -356,12 +356,30 @@ libxlDomainDefValidate(const virDomainDef *def,
>     return 0;
> }
>
>+static int
>+libxlDomainDeviceDefValidate(const virDomainDeviceDef *dev,
>+                             const virDomainDef *def,
>+                             void *opaque G_GNUC_UNUSED,
>+                             void *parseOpaque G_GNUC_UNUSED)
>+{
>+    if (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->filter) {
>+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+                       _("filterref is not supported in %1$s"),
>+                       virDomainVirtTypeToString(def->virtType));
>+        return -1;
>+    }
>+
>+    return 0;
>+}
>+
>+
> virDomainDefParserConfig libxlDomainDefParserConfig = {
>     .macPrefix = { 0x00, 0x16, 0x3e },
>     .netPrefix = LIBXL_GENERATED_PREFIX_XEN,
>     .devicesPostParseCallback = libxlDomainDeviceDefPostParse,
>     .domainPostParseCallback = libxlDomainDefPostParse,
>     .domainValidateCallback = libxlDomainDefValidate,
>+    .deviceValidateCallback = libxlDomainDeviceDefValidate,
>
>     .features = VIR_DOMAIN_DEF_FEATURE_USER_ALIAS |
>                 VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT |
>@@ -1460,6 +1478,10 @@ libxlDomainStartNew(libxlDriverPrivate *driver,
>                      managed_save_path);
>
>         vm->hasManagedSave = false;
>+    } else {
>+        /* Validate configuration if starting a new VM */
>+        if (virDomainDefValidate(vm->def, 0, driver->xmlopt, NULL) < 0)
>+            goto cleanup;
>     }
>
>     ret = libxlDomainStart(driver, vm, start_paused, restore_fd, restore_ver);
>-- 
>2.35.3
>