On 9/12/24 01:37, Peter Krempa wrote:
On Wed, Sep 11, 2024 at 18:24:07 -0400, Laine Stump wrote:
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@xxxxxxxx>
---
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)
The main difference between the two is that 'postParse' is called on
every parse of a XML. That means also for parsing the XMLs of
persistently defined domains.
If you reject parsing of a XML in a 'postParse' it fails to load the
persistent definition so the VM "vanishes", which is something we don't
want to do.
Nod. I had already seen that in my testing. Not nice, but I was prepared to
swallow the pill.
Thus doing validation in postParse is really valid only when it's a new
attribute or configuration that can't exist in "defined" state.
In contrast the validate callback is applied in a limited set of XML
entrypoints which then don't make the VM to vanish and certain other
situations. For that reason the 'Validate' callback/step needs to be
re-tried when starting the VM.
The validate callback plus a similar check at VM start is much nicer! I'll add
that to V2. Thanks!
Regards,
Jim
