Re: [PATCH 2/2] libxl: Reject VM config referencing nwfilters
On 9/11/24 5:02 PM, Jim Fehlig via Devel wrote:
The Xen libxl driver does not support nwfilter. Add a check for
nwfilters
to the devicesPostParseCallback, returning VIR_ERR_CONFIG_UNSUPPORTED if
any are found.
It's generally preferred for drivers to ignore unsupported XML features,
I would instead characterize it as "drivers generally ignore
*unrecognized* XML", but it's quite common for a bit of XML that's
understood and supported in one context within libvirt to generate an
UNSUPPORTED error when attempting to use it in a place where it isn't
supported.
but ignoring a user's request to filter VM network traffic can be
viewed
as a security issue.
Definitely.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_domain.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 0f129ec69c..2f6cebb8ae 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -131,6 +131,13 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDef *dev,
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;
+ }
+
This more properly should be in a function called
libxlValidateDomainDeviceDef(), which would look something like
qemuValidateDomainDeviceDef() and be added into
libxlDomainDefParserConfig with this initialization:
.deviceValidateCallback = libxlValidateDomainDeviceDef,
(my understanding of the purpose of the two has always been that the
PostParse callback is intended for performing post-parse
modifications/fixups to the domain def, while the Validate only checks
for correctness, without modifying anything. There are multiple cases of
having validation in the postparse function, but I think those are the
1) leftovers from before the introduction of the validate callbacks, and
2) the result of misunderstanding and/or sloth (e.g. in cases where you
want to validate something, but the driver you're adding this validation
to doesn't already have a deviceValidateCallback)
if (dev->type == VIR_DOMAIN_DEVICE_CHR &&
dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE
&&
dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE
&&