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

[Daniel P. Berrange]

On Tue, Oct 13, 2009 at 12:57:24PM +0200, Gerhard Stenzel wrote:
> +static int
> +networkAllowMacOnPort(virConnectPtr conn,
> +               struct qemud_driver *driver,
> +               char * ifname,
> +               unsigned char * mac) {
> +
> +    int err;
> +    char *macaddr;
> +
> +    if (virAsprintf(&macaddr,
> +                    "%02x:%02x:%02x:%02x:%02x:%02x",
> +                    mac[0], mac[1],
> +                    mac[2], mac[3],
> +                    mac[4], mac[5]) < 0) {
> +       virReportOOMError(conn);
> +       return -1;
> +    }
> +    /* allow this combination of macaddr and ifname */
> +
> +    ebtablesContext * ebtablescontext = driver->ebtables;
> +    if ((err = ebtablesAddForwardAllowIn(ebtablescontext,
> +                                         ifname,
> +                                         macaddr))) {
> +        virReportSystemError(conn, err,
> +                             _("failed to add ebtables rule to allow routing to '%s'"),
> +                             ifname);
> +    }

I think it'd be good if the 'ebtablesAddForwardAllowIn' method
actually too a  'unsigned char * mac'  array, and did the
conversion to printable string format itself. That would reduce
the amount of work the QEMU driver needs todo in this case, which
would be beneficial if we later add this MAC filtering to other
drivers like LXC, UML, etc

> +
> +    return 0;
> +}
> +
> +
> +int
>  qemudNetworkIfaceConnect(virConnectPtr conn,
>                           struct qemud_driver *driver,
>                           virDomainNetDefPtr net,
> @@ -1193,6 +1270,14 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
>          tapfd = -1;
>      }
>  
> +    if (driver->macFilter) {
> +        if ((err = networkAllowMacOnPort(conn, driver, net->ifname, net->mac))) {
> +            virReportSystemError(conn, err,
> +                                 _("failed to add ebtables rule to allow MAC address on  '%s'"),
> +                                 net->ifname);
> +        }
> +    }
> +
>  cleanup:
>      VIR_FREE(brname);
>  
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index f9a970f..3074ad1 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -26,6 +26,7 @@
>  
>  #include <config.h>
>  
> +#include "ebtables.h"
>  #include "internal.h"
>  #include "bridge.h"
>  #include "capabilities.h"
> @@ -112,6 +113,9 @@ struct qemud_driver {
>      char *hugetlbfs_mount;
>      char *hugepage_path;
>  
> +    unsigned int macFilter : 1;
> +    ebtablesContext *ebtables;
> +
>      virCapsPtr caps;
>  
>      /* An array of callbacks */
> @@ -205,4 +209,8 @@ virDomainDefPtr qemuParseCommandLineString(virConnectPtr conn,
>                                             virCapsPtr caps,
>                                             const char *args);
>  
> +
> +int networkDisableAllFrames(struct qemud_driver *driver);
> +int networkAddEbtablesRules(struct qemud_driver *driver);
> +
>  #endif /* __QEMUD_CONF_H */
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 3812f91..8abb19a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2164,6 +2164,37 @@ cleanup:
>      return -1;
>  }
>  
> +static int
> +networkDisallowMacOnPort(virConnectPtr conn,
> +               struct qemud_driver *driver,
> +               char * ifname,
> +               unsigned char * mac) {
> +
> +    int err;
> +    char *macaddr;
> +
> +    if (virAsprintf(&macaddr,
> +                    "%02x:%02x:%02x:%02x:%02x:%02x",
> +                    mac[0], mac[1],
> +                    mac[2], mac[3],
> +                    mac[4], mac[5]) < 0) {
> +       virReportOOMError(conn);
> +       return -1;
> +    }
> +    /* disallow this combination of macaddr and ifname */
> +
> +    ebtablesContext * ebtablescontext = driver->ebtables;
> +    if ((err = ebtablesRemoveForwardAllowIn(ebtablescontext,
> +                                         ifname,
> +                                         macaddr))) {
> +        virReportSystemError(conn, err,
> +                             _("failed to add ebtables rule to allow routing to '%s'"),
> +                             ifname);
> +    }
> +
> +    return 0;
> +}

Similar thought here - we should just pass the raw mac address
into ebtablesRemoveForwardAllowIn() and have convert to printable
format.

> +
>  
>  static void qemudShutdownVMDaemon(virConnectPtr conn,
>                                    struct qemud_driver *driver,
> @@ -2176,6 +2207,19 @@ static void qemudShutdownVMDaemon(virConnectPtr conn,
>  
>      VIR_DEBUG(_("Shutting down VM '%s'\n"), vm->def->name);
>  
> +    if (driver->macFilter) {
> +        int i;
> +        virDomainDefPtr def = vm->def;
> +        for (i = 0 ; i < def->nnets ; i++) {
> +            virDomainNetDefPtr net = def->nets[i];
> +            if ((errno = networkDisallowMacOnPort(conn, driver, net->ifname, net->mac))) {

NB, here you should add a 

    if (net->ifname == NULL)
       continue;

since, not all networking modes have a 'ifname' present

> +                virReportSystemError(conn, errno,
> +                                     _("failed to remove ebtables rule to allow MAC address on  '%s'"),
> +                                     net->ifname);
> +            }
> +        }
> +    }
> +


All in all, this code looks much nicer now QEMU is calling the  ebtables
APIs directly

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 :|