[libvirt] [PATCH v3 13/20] conf: report an error if nic needs filtering by no driver is present

[John Ferlan]

$SUBJ:

s/by/but


On 06/14/2018 08:33 AM, Daniel P. Berrangé wrote:
> If a <interface> includes a filter name but the nwfilter driver is not
> present we silently do nothing. This is very bad, because an application
> that thinks it is protected by malicious guest traffic will in fact be
> vulnerable. Reporting an error gives the administrator the ability to
> know there is a problem and fix it.
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  src/conf/domain_nwfilter.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

However/But/FWIW:
I know this method changes again in patch 20; however, it's easier to
describe now... All callers except 1 will only call here if net->filter
is true. The one outlier is the error path in qemuDomainChangeNetFilter
which will call with olddev before checking if olddev->filter was true.

Looking at qemuDomainChangeNet the STRNEQ_NULLABLE is used on
olddev->filter and newdev->filter, so I think we could currently end up
in a fairly strange place - although there will be errors eventually
from virNWFilterInstantiateFilterUpdate when @filtername is not found.

I'm tempted to suggest moving the net->filter check into here, but I
know that by patch 20 all that changes again. It's fine to do things
later, but figured while it was fresh in my mind - I'd note it.

> diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c
> index e360aceeba..7570e0ae83 100644
> --- a/src/conf/domain_nwfilter.c
> +++ b/src/conf/domain_nwfilter.c
> @@ -28,6 +28,9 @@
>  #include "datatypes.h"
>  #include "domain_conf.h"
>  #include "domain_nwfilter.h"
> +#include "virerror.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_NWFILTER
>  
>  static virDomainConfNWFilterDriverPtr nwfilterDriver;
>  
> @@ -44,8 +47,10 @@ virDomainConfNWFilterInstantiate(const char *vmname,
>  {
>      if (nwfilterDriver != NULL)
>          return nwfilterDriver->instantiateFilter(vmname, vmuuid, net);
> -    /* driver module not available -- don't indicate failure */
> -    return 0;
> +
> +    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                   _("No network filter driver available"));
> +    return -1;
>  }
>  
>  void
>