[PATCH V2] libxl: Reject VM config referencing nwfilters

Jim Fehlig via Devel posted 1 patch 3 weeks, 2 days ago
src/libxl/libxl_conf.c   |  7 +++++++
src/libxl/libxl_domain.c | 18 ++++++++++++++++++
2 files changed, 25 insertions(+)
[PATCH V2] libxl: Reject VM config referencing nwfilters
Posted by Jim Fehlig via Devel 3 weeks, 2 days 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>
---

This is a V2 of patch2 from this series

https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/QDRDSKDLL5GZVXDSIJO5R32Q5F4AFZLR/

I've pushed patch1. Personally I'm fine leaving it at that, but I
made it this far so might as well give patch2 another attempt :-).
There's still the open question whether the same should be done for
the other hypervisor drivers that do not support nwfilters.

Changes in V2:
Use deviceValidateCallback instead of devicesPostParseCallback
Reject use of nwfilters at VM start

 src/libxl/libxl_conf.c   |  7 +++++++
 src/libxl/libxl_domain.c | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 62e1be6672..bf5d925a20 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1279,6 +1279,13 @@ libxlMakeNic(virDomainDef *def,
      * x_nics[i].mtu = 1492;
      */
 
+    if (l_nic->filter) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("filterref is not supported in %1$s"),
+                       virDomainVirtTypeToString(def->virtType));
+        return -1;
+    }
+
     if (l_nic->script && !(actual_type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
                            actual_type == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 0f129ec69c..d400f32627 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 |
-- 
2.35.3
Re: [PATCH V2] libxl: Reject VM config referencing nwfilters
Posted by Peter Krempa 3 weeks, 1 day ago
On Thu, Sep 12, 2024 at 16:04:44 -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>
> ---
> 
> This is a V2 of patch2 from this series
> 
> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/QDRDSKDLL5GZVXDSIJO5R32Q5F4AFZLR/
> 
> I've pushed patch1. Personally I'm fine leaving it at that, but I
> made it this far so might as well give patch2 another attempt :-).
> There's still the open question whether the same should be done for
> the other hypervisor drivers that do not support nwfilters.
> 
> Changes in V2:
> Use deviceValidateCallback instead of devicesPostParseCallback
> Reject use of nwfilters at VM start
> 
>  src/libxl/libxl_conf.c   |  7 +++++++
>  src/libxl/libxl_domain.c | 18 ++++++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 62e1be6672..bf5d925a20 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1279,6 +1279,13 @@ libxlMakeNic(virDomainDef *def,
>       * x_nics[i].mtu = 1492;
>       */
>  
> +    if (l_nic->filter) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("filterref is not supported in %1$s"),
> +                       virDomainVirtTypeToString(def->virtType));
> +        return -1;
> +    }

As noted in my reply to the other thread, rather than adding this
duplicated check you should simply ensure that the validate
infrastructure is called at the startup time of any VM as we do in the
qemu driver.

Calling:

    if (virDomainDefValidate(vm->def, 0, driver->xmlopt, qemuCaps) < 0)
        return -1;

in a code path that all VM startup takes should do the trick. I'm not
sure though what the libxl driver passes as the opaque data for the
validation callbacks though.