[libvirt] [RFC PATCH 5/6] add MAC address based port filtering to qemu

[Daniel P. Berrange]

On Fri, Oct 02, 2009 at 03:48:36PM +0200, Gerhard Stenzel wrote:
> This patch adds MAC address based port filtering to the qemu driver.
> 
> Signed-off-by: Gerhard Stenzel <gerhard.stenzel at de.ibm.com>
> ---
> 
>  src/qemu/qemu.conf     |    3 +++
>  src/qemu/qemu_conf.c   |   14 ++++++++++++++
>  src/qemu/qemu_conf.h   |    2 ++
>  src/qemu/qemu_driver.c |   23 +++++++++++++++++++++++
>  4 files changed, 42 insertions(+), 0 deletions(-)


> @@ -1193,6 +1197,16 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
>          tapfd = -1;
>      }
>  
> +    if (driver->macFilter) {
> +        virNetworkPtr network = virNetworkLookupByName(conn,
> +                                                       net->data.network.name);
> +        if ((err = virNetworkAllowMacOnPort(network, brname, net->ifname, net->mac))) {
> +            virReportSystemError(conn, err,
> +                                 _("failed to add ebtables rule to allow MAC address on  '%s'"),
> +                                 net->ifname);
> +        }
> +    }

This will crash in a large number of scenarios, since it is 
only valid to deference net->data.network  fields once you
have verified net->type == VIR_DOMAIN_NET_TYPE_NETWORK. It
is also failing to check for virNetworkLookupByName() returning
NULL.

This is why the MAC filtering should not be part of the 
virNetwork  API set. The QEMU driver should be directly
calling the ebtables APIs you added in patch 1, rather
then indirectly via virNetwork.. This would allow this
MAC filtering to work with bridged network modes too.

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 155e4a3..a95c867 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -239,6 +239,14 @@ qemudAutostartConfigs(struct qemud_driver *driver) {
>          }
>          virDomainObjUnlock(vm);
>      }
> +    if (qemu_driver->macFilter) {
> +        fprintf(stderr,"%s:%s:%d macFilter=%d\n", __FILE__, __FUNCTION__, __LINE__, qemu_driver->macFilter);

There's a   VIR_DEBUG()  macro available for logging.


> +        if ((errno = virNetworkDisableAllFrames(conn))) {
> +            virReportSystemError(conn, errno,
> +                                 _("failed to add rule to drop all frames in '%s'"), __FILE__);
> +        }
> +    }
> +
>      qemuDriverUnlock(driver);
>  
>      if (conn)
> @@ -2167,8 +2175,23 @@ cleanup:
>  static void qemudShutdownVMDaemon(virConnectPtr conn,
>                                    struct qemud_driver *driver,
>                                    virDomainObjPtr vm) {
> +
>      int ret;
>      int retries = 0;
> +    char *brname;
> +
> +    virDomainNetDefPtr net =  vm->def->nets[0];

This assumes the guest has exactly one NIC - it'll crash if there
are no NICs, and it'll miss cleanup steps if there are multiple NICs

> +    virNetworkPtr network = virNetworkLookupByName(conn,
> +                                                   net->data.network.name);
> +    brname = virNetworkGetBridgeName(network);
> +
> +    if (driver->macFilter) {
> +        if ((errno = virNetworkDisallowMacOnPort(network, brname, net->ifname, net->mac))) {
> +            virReportSystemError(conn, errno,
> +                                 _("failed to add ebtables rule to allow MAC address on  '%s'"),
> +                                 net->ifname);
> +        }
> +    }

Same comment as before about not using virNetwork for any of this

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|