On Wed, Sep 11, 2024 at 05:09:03PM -0600, Jim Fehlig wrote:
> On 9/11/24 16:24, 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,
>
> Yes, good point! The libxl driver already has domainValidateCallback, but
> now needs a deviceValidateCallback for this code. I'll make that change in
> V2.
>
> Before sending another version, I'd like to hear opinions on Demi's question
> about the other hypervisor drivers. Do they need a similar change?
I'm not familiar with how libvirt works internally, but to me it seems
like one option would be to have a flag set on the drivers that support
network filtering. Generic code would then check the flag and fail if
filtering is requested with a driver that doesn't support it.
This has the advantage of not requiring changes to each and every
driver.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
Attachment:
signature.asc
Description: PGP signature
