[libvirt] [libvirt PATCHv5 1/2] add DHCP snooping

Stefan Berger stefanb at linux.vnet.ibm.com
Wed Nov 16 13:12:30 UTC 2011


On 11/10/2011 03:28 PM, David L Stevens wrote:
> This patch adds DHCP Snooping support to libvirt.
>
> Signed-off-by: David L Stevens<dlstevens at us.ibm.com>
> ---
>   docs/formatnwfilter.html.in              |   17 +
>   examples/xml/nwfilter/no-ip-spoofing.xml |    5 +
>   src/Makefile.am                          |    2 +
>   src/nwfilter/nwfilter_dhcpsnoop.c        |  745 ++++++++++++++++++++++++++++++
>   src/nwfilter/nwfilter_dhcpsnoop.h        |   38 ++
>   src/nwfilter/nwfilter_driver.c           |    6 +
>   src/nwfilter/nwfilter_gentech_driver.c   |   53 ++-
>   7 files changed, 853 insertions(+), 13 deletions(-)
>   create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.c
>   create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.h
>
> diff --git a/docs/formatnwfilter.html.in b/docs/formatnwfilter.html.in
> index 8df4a93..8003320 100644
> --- a/docs/formatnwfilter.html.in
> +++ b/docs/formatnwfilter.html.in
> @@ -1775,6 +1775,23 @@
>          <br/><br/>
>          In case a VM is resumed after suspension or migrated, IP address
>          detection will be restarted.
> +<br/><br/>
> +       Variable<i>ip_learning</i>  may be used to specify
> +       the IP address learning method. Valid values are<i>any</i>,<i>dhcp</i>,
> +       or<i>none</i>. The default value is<i>any</i>, meaning that libvirt
> +       may use any packet to determine the address in use by a VM. A value of
> +<i>dhcp</i>  specifies that libvirt should only honor DHCP server-assigned
> +       addresses with valid leases. If<i>ip_learning</i>  is set to<i>none</i>,
> +       libvirt does not do address learning and referencing<i>IP</i>  without
> +       assigning it an explicit value is an error.
> +<br/><br/>
> +       Use of<i>ip_learning=dhcp</i>  (DHCP snooping) provides additional
> +       anti-spoofing security, especially when combined with a filter allowing
> +       only trusted DHCP servers to assign addresses. If the DHCP lease expires,
> +       the VM will no longer be able to use the IP address until it acquires a
> +       new, valid lease from a DHCP server. If the VM is migrated, it must get
> +       a new valid DHCP lease to use an IP address (e.g., by
> +       bringing the VM interface down and up again).
>        </p>
>
Can you add a sentence that it must be used in combination with a filter 
that allows DHCP traffic to pass?
>       <h3><a name="nwflimitsmigr">VM Migration</a></h3>
> diff --git a/examples/xml/nwfilter/no-ip-spoofing.xml b/examples/xml/nwfilter/no-ip-spoofing.xml
> index b8c94c8..7c7fd46 100644
> --- a/examples/xml/nwfilter/no-ip-spoofing.xml
> +++ b/examples/xml/nwfilter/no-ip-spoofing.xml
> @@ -1,5 +1,10 @@
>   <filter name='no-ip-spoofing' chain='ipv4'>
>
> +<!-- allow DHCP requests -->
> +<rule action='accept' direction='out'>
> +<ip match='yes' srcipaddr='0.0.0.0' protocol='udp' srcportstart='68'
> +                  srcportend='68' />
> +</rule>
>       <!-- drop if srcipaddr is not the IP address of the guest -->
>       <rule action='drop' direction='out'>
>           <ip match='no' srcipaddr='$IP' />
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 87d91ed..c948cbf 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -481,6 +481,8 @@ NWFILTER_DRIVER_SOURCES =					\
>   		nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c	\
>   		nwfilter/nwfilter_gentech_driver.c			\
>   		nwfilter/nwfilter_gentech_driver.h			\
> +		nwfilter/nwfilter_dhcpsnoop.c				\
> +		nwfilter/nwfilter_dhcpsnoop.h				\
>   		nwfilter/nwfilter_ebiptables_driver.c			\
>   		nwfilter/nwfilter_ebiptables_driver.h			\
>   		nwfilter/nwfilter_learnipaddr.c				\
> diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
> new file mode 100644
> index 0000000..8a37a6f
> --- /dev/null
> +++ b/src/nwfilter/nwfilter_dhcpsnoop.c
> @@ -0,0 +1,745 @@
> +/*
> + * nwfilter_dhcpsnoop.c: support for DHCP snooping used by a VM
> + *                         on an interface
> + *
> + * Copyright (C) 2011 IBM Corp.
> + * Copyright (C) 2011 David L Stevens
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
> + *
> + * Author: David L Stevens<dlstevens at us.ibm.com>
> + * Based in part on work by Stefan Berger<stefanb at us.ibm.com>
> + */
> +
> +#include<config.h>
> +
> +#ifdef HAVE_LIBPCAP
> +#include<pcap.h>
> +#endif
> +
> +#include<fcntl.h>
> +#include<sys/ioctl.h>
> +#include<signal.h>
> +
> +#include<arpa/inet.h>
> +#include<net/ethernet.h>
> +#include<netinet/ip.h>
> +#include<netinet/udp.h>
> +#include<net/if.h>
> +#include<net/if_arp.h>
> +#include<intprops.h>
> +
> +#include "internal.h"
> +
> +#include "buf.h"
> +#include "memory.h"
> +#include "logging.h"
> +#include "datatypes.h"
> +#include "interface.h"
> +#include "virterror_internal.h"
> +#include "threads.h"
> +#include "conf/nwfilter_params.h"
> +#include "conf/domain_conf.h"
> +#include "nwfilter_gentech_driver.h"
> +#include "nwfilter_ebiptables_driver.h"
> +#include "nwfilter_dhcpsnoop.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_NWFILTER
> +
> +#ifdef HAVE_LIBPCAP
> +
> +static virHashTablePtr SnoopReqs;
> +static pthread_mutex_t SnoopLock;
> +
> +#define snoop_lock()    { pthread_mutex_lock(&SnoopLock); }
> +#define snoop_unlock()  { pthread_mutex_unlock(&SnoopLock); }
> +
> +struct virNWFilterSnoopReq {
> +    virNWFilterTechDriverPtr  techdriver;
> +    const char               *ifname;
> +    int                       ifindex;
> +    const char               *linkdev;
> +    enum virDomainNetType     nettype;
> +    unsigned char             macaddr[VIR_MAC_STRING_BUFLEN];
> +    const char               *filtername;
> +    virNWFilterHashTablePtr   vars;
> +    virNWFilterDriverStatePtr driver;
> +    /* start and end of lease list, ordered by lease time */
> +    struct iplease           *start;
> +    struct iplease           *end;
> +    bool                      die;
> +};
> +
> +#define POLL_INTERVAL    10*1000 /* 10 secs */
> +
> +struct iplease {
> +    uint32_t                    ipl_ipaddr;
> +    uint32_t                    ipl_server;
> +    struct virNWFilterSnoopReq *ipl_req;
> +    unsigned int                ipl_timeout;
> +    /* timer list */
> +    struct iplease             *ipl_prev;
> +    struct iplease             *ipl_next;
> +};
> +
> +static struct iplease *ipl_getbyip(struct iplease *start, uint32_t ipaddr);
> +static void ipl_update(struct iplease *pl, uint32_t timeout);
> +
> +
> +/*
> + * ipl_ladd - add an IP lease to a list
> + */
> +static void
> +ipl_ladd(struct iplease *plnew, struct iplease **start, struct iplease **end)
> +{
> +    struct iplease             *pl;
> +
> +    plnew->ipl_next = plnew->ipl_prev = 0;
> +    if (!*start) {
> +        *start = *end = plnew;
> +        return;
> +    }
> +    for (pl = *end; pl&&  plnew->ipl_timeout<  pl->ipl_timeout;
> +         pl = pl->ipl_prev)
> +        /* empty */ ;
> +    if (!pl) {
> +        plnew->ipl_next = *start;
> +        *start = plnew;
> +    } else {
> +        plnew->ipl_next = pl->ipl_next;
> +        pl->ipl_next = plnew;
> +    }
> +    plnew->ipl_prev = pl;
> +    if (plnew->ipl_next)
> +        plnew->ipl_next->ipl_prev = plnew;
> +    else
> +        *end = plnew;
> +}
> +
> +/*
> + * ipl_tadd - add an IP lease to the timer list
> + */
> +static void
> +ipl_tadd(struct iplease *plnew)
> +{
> +    struct virNWFilterSnoopReq *req = plnew->ipl_req;
> +
> +    ipl_ladd(plnew,&req->start,&req->end);
> +}
> +
> +/*
> + * ipl_install - install rule for a lease
> + */
> +static int
> +ipl_install(struct iplease *ipl)
> +{
> +    char            ipbuf[20];             /* dotted decimal IP addr string */
> +    char           *ipstr;
> +    int             rc;
> +
> +    if (!inet_ntop(AF_INET,&ipl->ipl_ipaddr, ipbuf, sizeof(ipbuf))) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("ipl_install inet_ntop " "failed (0x%08X)"),
> +                               ipl->ipl_ipaddr);
> +        return -1;
> +    }
> +
> +    ipstr = strdup(ipbuf);
> +    if (!ipstr) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +    if (virNWFilterHashTablePut(ipl->ipl_req->vars, "IP", ipstr, 1)) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Could not add variable \"IP\" to hashmap"));
> +        VIR_FREE(ipstr);
> +        return -1;
> +    }
> +    rc = virNWFilterInstantiateFilterLate(NULL,
> +                                          ipl->ipl_req->ifname,
> +                                          ipl->ipl_req->ifindex,
> +                                          ipl->ipl_req->linkdev,
> +                                          ipl->ipl_req->nettype,
> +                                          ipl->ipl_req->macaddr,
> +                                          ipl->ipl_req->filtername,
> +                                          ipl->ipl_req->vars,
> +                                          ipl->ipl_req->driver);
> +    if (rc) {
> +        VIR_FREE(ipstr);
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +/*
> + * ipl_add - create or update an IP lease
> + */
> +static void
> +ipl_add(struct iplease *plnew)
> +{
> +    struct iplease *pl;
> +    struct virNWFilterSnoopReq *req = plnew->ipl_req;
> +
> +    pl = ipl_getbyip(req->start, plnew->ipl_ipaddr);
> +    if (pl) {
> +        ipl_update(pl, plnew->ipl_timeout);
> +        return;
> +    }
> +    /* support for multiple addresses requires the ability to add filters
> +     * to existing chains, or to instantiate address lists via
> +     * virNWFilterInstantiateFilterLate(). Until one of those capabilities
> +     * is added, don't allow a new address when one is already assigned to
> +     * this interface.
> +     */
> +    if (req->start)
> +         return;    /* silently ignore multiple addresses */
> +
> +    if (VIR_ALLOC(pl)<  0) {
> +        virReportOOMError();
> +        return;
> +    }
> +    *pl = *plnew;
> +
> +    if (ipl_install(pl)<  0) {
> +        VIR_FREE(pl);
> +        return;
> +    }
> +    ipl_tadd(pl);
> +}
> +
> +/*
> + * ipl_tdel - remove an IP lease from a list
> + */
> +static void
> +ipl_ldel(struct iplease *ipl, struct iplease **start, struct iplease **end)
> +{
> +    if (ipl->ipl_prev)
> +        ipl->ipl_prev->ipl_next = ipl->ipl_next;
> +    else
> +        *start = ipl->ipl_next;
> +    if (ipl->ipl_next)
> +        ipl->ipl_next->ipl_prev = ipl->ipl_prev;
> +    else
> +        *end = ipl->ipl_prev;
> +    ipl->ipl_next = ipl->ipl_prev = 0;
> +}
> +
> +/*
> + * ipl_tdel - remove an IP lease from the timer list
> + */
> +static void
> +ipl_tdel(struct iplease *ipl)
> +{
> +    struct virNWFilterSnoopReq *req = ipl->ipl_req;
> +
> +    ipl_ldel(ipl,&req->start,&req->end);
> +    ipl->ipl_timeout = 0;
> +}
> +
> +/*
> + * ipl_del - delete an IP lease
> + */
> +static void
> +ipl_del(struct virNWFilterSnoopReq *req, uint32_t ipaddr)
> +{
> +    struct iplease *ipl;
> +
> +    ipl = ipl_getbyip(req->start, ipaddr);
> +    if (ipl == NULL)
> +        return;
> +
> +    ipl_tdel(ipl);
> +
> +    /* for multiple address support, this needs to remove those rules
> +     * referencing "IP" with ipl's ip value.
> +     */
> +    if (req->techdriver->applyDHCPOnlyRules(req->ifname, req->macaddr, NULL)) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "ipl_ldel failed");
> +    }
> +    VIR_FREE(ipl);
> +}
> +
> +/*
> + * ipl_update - update the timeout on an IP lease
> + */
> +static void
> +ipl_update(struct iplease *ipl, uint32_t timeout)
> +{
> +    if (timeout<  ipl->ipl_timeout)
> +        return;  /* no take-backs */
> +    ipl_tdel(ipl);
> +    ipl->ipl_timeout = timeout;
> +    ipl_tadd(ipl);
> +    return;
> +}
> +
> +/*
> + * ipl_getbyip - lookup IP lease by IP address
> + */
> +static struct iplease *
> +ipl_getbyip(struct iplease *start, uint32_t ipaddr)
> +{
> +    struct iplease *pl;
> +
> +    for (pl = start; pl&&  pl->ipl_ipaddr != ipaddr; pl = pl->ipl_next)
> +        /* empty */ ;
> +    return pl;
> +}
> +
> +/*
> + * ipl_trun - run the IP lease timeout list
> + */
> +static unsigned int
> +ipl_trun(struct virNWFilterSnoopReq *req)
> +{
> +    uint32_t now;
> +
> +    now = time(0);
> +    while (req->start&&  req->start->ipl_timeout<= now)
> +        ipl_del(req, req->start->ipl_ipaddr);
> +    return 0;
> +}
> +
> +typedef unsigned char Eaddr[6];
> +
> +struct eth {
> +    Eaddr eh_dst;
> +    Eaddr eh_src;
> +    unsigned short eh_type;
> +    union {
> +        unsigned char eu_data[0];
> +        struct vlan_hdr {
> +            unsigned short ev_flags;
> +            unsigned short ev_type;
> +            unsigned char  ev_data[0];
> +        } eu_vlh;
> +    } eth_un;
> +} ATTRIBUTE_PACKED;
> +
> +#define eh_data eth_un.eu_data
> +#define ehv_data eth_un.eu_vlh.ev_data
> +#define ehv_type eth_un.eu_vlh.ev_type
> +
> +struct dhcp {
> +    unsigned char  d_op;
> +    unsigned char  d_htype;
> +    unsigned char  d_hlen;
> +    unsigned char  d_hops;
> +    unsigned int   d_xid;
> +    unsigned short d_secs;
> +    unsigned short d_flags;
> +    unsigned int   d_ciaddr;
> +    unsigned int   d_yiaddr;
> +    unsigned int   d_siaddr;
> +    unsigned int   d_giaddr;
> +    unsigned char  d_chaddr[16];
> +    char           d_sname[64];
> +    char           d_file[128];
> +    unsigned char  d_opts[0];
> +};
> +
> +/* DHCP options */
> +
> +#define DHCPO_PAD         0
> +#define DHCPO_LEASE      51     /* lease time in secs */
> +#define DHCPO_MTYPE      53     /* message type */
> +#define DHCPO_END       255     /* end of options */
> +
> +/* DHCP message types */
> +#define DHCPDECLINE     4
> +#define DHCPACK         5
> +#define DHCPRELEASE     7
> +
> +unsigned char dhcp_magic[4] = { 99, 130, 83, 99 };
> +
> +static int
> +dhcp_getopt(struct dhcp *pd, int len, int *pmtype, int *pleasetime)
> +{
> +    int oind, olen;
> +    int oend;
> +
> +    olen = len - sizeof *pd;
> +    oind = 0;
> +
> +    if (olen<  4)               /* bad magic */
> +        return -1;
> +    if (memcmp(dhcp_magic, pd->d_opts, sizeof dhcp_magic) != 0)
> +        return -1;              /* bad magic */
> +    oind += sizeof dhcp_magic;
> +
> +    oend = 0;
> +
> +    *pmtype = *pleasetime = 0;
> +
> +    while (oind<  olen) {
> +        switch (pd->d_opts[oind]) {
> +            case DHCPO_LEASE:
> +                if (olen - oind<  6)
> +                    goto malformed;
> +                if (*pleasetime)
> +                    return -1;  /* duplicate lease time */
> +                *pleasetime =
> +                    ntohl(*(unsigned int *) (pd->d_opts + oind + 2));
> +                break;
> +            case DHCPO_MTYPE:
> +                if (olen - oind<  3)
> +                    goto malformed;
> +                if (*pmtype)
> +                    return -1;  /* duplicate message type */
> +                *pmtype = pd->d_opts[oind + 2];
> +                break;
> +            case DHCPO_PAD:
> +                oind++;
> +                continue;
> +
> +            case DHCPO_END:
> +                oend = 1;
> +                break;
> +            default:
> +                if (olen - oind<  2)
> +                    goto malformed;
> +        }
> +        if (oend)
> +            break;
> +        oind += pd->d_opts[oind + 1] + 2;
> +    }
> +    return 0;
> +  malformed:
> +    VIR_WARN("got lost in the options!");
> +    return -1;
> +}
> +
> +static void
> +dhcpdecode(struct virNWFilterSnoopReq *req, struct eth *pep, int len)
> +{
> +    struct iphdr   *pip;
> +    struct udphdr  *pup;
> +    struct dhcp    *pd;
> +    struct iplease  ipl;
> +    int             mtype, leasetime;
> +
> +    /* go through the protocol headers */
> +    switch (ntohs(pep->eh_type)) {
> +    case ETHERTYPE_IP:
> +        pip = (struct iphdr *) pep->eh_data;
> +        len -= offsetof(struct eth, eh_data);
> +        break;
> +    case ETHERTYPE_VLAN:
> +        if (ntohs(pep->ehv_type) != ETHERTYPE_IP)
> +            return;
> +        pip = (struct iphdr *) pep->ehv_data;
> +        len -= offsetof(struct eth, ehv_data);
> +        break;
Probably the previous discussions were a bit misleading. ebtables cannot 
filter ARP,RARP,ipv4 and IPv6 traffic when the VM is sending VLAN 
traffic. So we really have to ignore VLAN traffic and NOT instantiate 
any filters due to packets with VLAN headers since we won't be able to 
filter that later. I have a patch for that removing similar code in the 
ip learning code and have a section in limitations for the documentation 
-- all pending patches. I guess treat it like the 'default' case.
> +    default:
> +        return;
> +    }
> +    pip = (struct iphdr *) pep->eh_data;
> +    len -= sizeof(*pep);
> +    pup = (struct udphdr *) ((char *) pip + (pip->ihl<<  2));
> +    len -= pip->ihl<<  2;
> +    pd = (struct dhcp *) ((char *) pup + sizeof(*pup));
> +    len -= sizeof(*pup);
> +    if (len<  0)
> +        return;                 /* dhcpdecode: invalid packet length */
> +    if (dhcp_getopt(pd, len,&mtype,&leasetime)<  0)
> +        return;
> +
> +    memset(&ipl, 0, sizeof(ipl));
> +    ipl.ipl_ipaddr = pd->d_yiaddr;
> +    ipl.ipl_server = pd->d_siaddr;
> +    if (leasetime == ~0)
> +        ipl.ipl_timeout = ~0;
> +    else
> +        ipl.ipl_timeout = time(0) + leasetime;
> +    ipl.ipl_req = req;
> +
> +    switch (mtype) {
> +        case DHCPACK:
> +            ipl_add(&ipl);
> +            break;
> +        case DHCPDECLINE:
> +        case DHCPRELEASE:
> +            ipl_del(req, ipl.ipl_ipaddr);
> +            break;
> +        default:
> +            break;
> +    }
> +}
> +
> +#define PBUFSIZE        576     /*>= IP/TCP/DHCP headers */
> +
> +static pcap_t *
> +dhcpopen(const char *intf)
> +{
> +    pcap_t             *handle;
> +    struct bpf_program  fp;
> +    char                filter[64];
> +    char                pcap_errbuf[PCAP_ERRBUF_SIZE];
> +
> +    handle = pcap_open_live(intf, PBUFSIZE, 0, POLL_INTERVAL, pcap_errbuf);
> +    if (handle == NULL) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("pcap_open_live: %s"), pcap_errbuf);
> +        return 0;
> +    }
> +
> +    sprintf(filter, "port 67 or dst port 68");
Preferring snprintf.
> +    if (pcap_compile(handle,&fp, filter, 1, PCAP_NETMASK_UNKNOWN) != 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("pcap_compile: %s"), pcap_geterr(handle));
> +        return 0;
> +    }
> +    if (pcap_setfilter(handle,&fp) != 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("pcap_setfilter: %s"), pcap_geterr(handle));
> +        return 0;
> +    }
> +    pcap_freecode(&fp);
> +    return handle;
> +}
> +
> +static void
> +snoopReqFree(struct virNWFilterSnoopReq *req)
> +{
> +    struct iplease *ipl;
> +
> +    if (!req)
> +        return;
> +
> +    /* free all leases */
> +    snoop_lock();
> +    for (ipl = req->start; ipl; ipl = req->start)
> +        ipl_del(req, ipl->ipl_ipaddr);
> +    snoop_unlock();
> +
> +    /* free all req data */
> +    VIR_FREE(req->ifname);
> +    VIR_FREE(req->linkdev);
> +    VIR_FREE(req->filtername);
> +    virNWFilterHashTableFree(req->vars);
> +    VIR_FREE(req);
> +}
> +
> +static void *
> +virNWFilterDHCPSnoop(void *req0)
> +{
> +    struct virNWFilterSnoopReq *req = req0;
> +    pcap_t                     *handle;
> +    struct pcap_pkthdr          hdr;
> +    struct eth                 *packet;
> +    int                         ifindex;
> +
> +    handle = dhcpopen(req->ifname);
> +    if (!handle)
> +        return 0;
> +
> +    ifindex = if_nametoindex(req->ifname);
> +
> +    while (1) {
> +        if (req->die)
> +            break;
> +        snoop_lock();
> +        ipl_trun(req);
> +        snoop_unlock();
> +
> +        packet = (struct eth *) pcap_next(handle,&hdr);
> +
> +        if (!packet) {
> +            if (ifaceCheck(false, req->ifname, NULL, ifindex))
> +                virNWFilterDHCPSnoopEnd(req->ifname);
> +            continue;
> +        }
> +
> +        snoop_lock();
> +        dhcpdecode(req, packet, hdr.caplen);
> +        snoop_unlock();
> +    }
> +    snoopReqFree(req);
> +    return 0;
> +}
> +
> +int
> +virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
> +                        const char *ifname,
> +                        int ifindex,
> +                        const char *linkdev,
> +                        enum virDomainNetType nettype,
> +                        const unsigned char *macaddr,
> +                        const char *filtername,
> +                        virNWFilterHashTablePtr filterparams,
> +                        virNWFilterDriverStatePtr driver)
> +{
> +    struct virNWFilterSnoopReq *req;
> +    pthread_t                   thread;
> +
> +    snoop_lock();
> +    req = virHashLookup(SnoopReqs, ifname);
> +    snoop_unlock();
> +    if (req)
> +        return 0;
> +
> +    if (VIR_ALLOC(req)<  0) {
> +        virReportOOMError();
> +        return 1;
Ok, I'll convert these to a '-1' in a later cleanup patch.
> +    }
> +
> +    req->ifname = strdup(ifname);
> +    if (req->ifname == NULL) {
> +        snoopReqFree(req);
> +        virReportOOMError();
> +        return 1;
> +    }
> +
> +    req->techdriver = techdriver;
> +    req->ifindex = ifindex;
> +    req->linkdev = linkdev ? strdup(linkdev) : NULL;
> +    req->nettype = nettype;
> +    memcpy(req->macaddr, macaddr, sizeof(req->macaddr));
> +    req->filtername = strdup(filtername);
> +    if (req->filtername == NULL) {
> +        snoopReqFree(req);
> +        virReportOOMError();
> +        return 1;
> +    }
> +
> +    if (techdriver->applyDHCPOnlyRules(req->ifname, req->macaddr, NULL)) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "applyDHCPOnlyRules "
> +                               "failed - spoofing not protected!");
> +    }
> +
> +    req->vars = virNWFilterHashTableCreate(0);
> +    if (!req->vars) {
> +        snoopReqFree(req);
> +        virReportOOMError();
> +        return 1;
> +    }
> +    if (virNWFilterHashTablePutAll(filterparams, req->vars)) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("virNWFilterDHCPSnoopReq: can't copy variables"
> +                                " on if %s"), ifname);
> +        snoopReqFree(req);
> +        return 1;
> +    }
> +    req->driver = driver;
> +
> +    snoop_lock();
> +    if (virHashAddEntry(SnoopReqs, ifname, req)) {
> +        snoop_unlock();
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("virNWFilterDHCPSnoopReq req add failed on "
> +                                "interface \"%s\""), ifname);
> +        snoopReqFree(req);
> +        return 1;
> +    }
> +    snoop_unlock();
> +    if (pthread_create(&thread, NULL, virNWFilterDHCPSnoop, req) != 0) {
> +        (void) virHashRemoveEntry(SnoopReqs, ifname);
> +        snoopReqFree(req);
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("virNWFilterDHCPSnoopReq pthread_create failed"
> +                                " on interface \"%s\""), ifname);
> +        return 1;
> +    }
> +    return 0;
> +}
> +
> +/*
> + * freeReq - hash table free function to kill a request
> + */
> +static void
> +freeReq(void *req0, const void *name ATTRIBUTE_UNUSED)
> +{
> +    struct virNWFilterSnoopReq *req = (struct virNWFilterSnoopReq *) req0;
> +
> +    if (!req)
> +        return;
> +
> +    req->die = 1;
> +}
> +
> +int
> +virNWFilterDHCPSnoopInit(void)
> +{
> +    if (SnoopReqs)
> +        return 0;
> +
> +    if (pthread_mutex_init(&SnoopLock, 0)<  0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("pthread_mutex_init: %s"), strerror(errno));
> +        return -1;
> +    }
> +    snoop_lock();
> +    SnoopReqs = virHashCreate(0, freeReq);
> +    if (!SnoopReqs) {
> +        snoop_unlock();
> +        virReportOOMError();
> +        return -1;
> +    }
> +    snoop_unlock();
> +    return 0;
> +}
> +
> +void
> +virNWFilterDHCPSnoopEnd(const char *ifname)
> +{
> +    snoop_lock();
> +    if (!SnoopReqs) {
> +        snoop_unlock();
> +        return;
> +    }
> +    if (ifname)
> +        virHashRemoveEntry(SnoopReqs, ifname);
> +    else {                      /* free all of them */
Following coding style, the if branch should have a '{'.
> +        virHashFree(SnoopReqs);
> +        SnoopReqs = virHashCreate(0, freeReq);
> +        if (!SnoopReqs) {
> +            snoop_unlock();
> +            virReportOOMError();
> +            return;
> +        }
> +    }
> +    snoop_unlock();
> +}
> +
> +#else /* HAVE_LIBPCAP */
> +int
> +virNWFilterDHCPSnoopInit(void)
> +{
> +    return -1;
> +}
> +
> +void
> +virNWFilterDHCPSnoopEnd(const char *ifname ATTRIBUTE_UNUSED)
> +{
> +    return;
> +}
> +
> +int
> +virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver ATTRIBUTE_UNUSED,
> +                        const char *ifname ATTRIBUTE_UNUSED,
> +                        int ifindex ATTRIBUTE_UNUSED,
> +                        const char *linkdev ATTRIBUTE_UNUSED,
> +                        enum virDomainNetType nettype ATTRIBUTE_UNUSED,
> +                        const unsigned char *macaddr ATTRIBUTE_UNUSED,
> +                        const char *filtername ATTRIBUTE_UNUSED,
> +                        virNWFilterHashTablePtr filterparams ATTRIBUTE_UNUSED,
> +                        virNWFilterDriverStatePtr driver ATTRIBUTE_UNUSED)
> +{
> +    virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("libvirt was not compiled "
> +                           "with libpcap and \"ip_learning='dhcp'\" requires"
> +                           " it."));
> +    return 1;
> +}
> +#endif /* HAVE_LIBPCAP */
> diff --git a/src/nwfilter/nwfilter_dhcpsnoop.h b/src/nwfilter/nwfilter_dhcpsnoop.h
> new file mode 100644
> index 0000000..94762d1
> --- /dev/null
> +++ b/src/nwfilter/nwfilter_dhcpsnoop.h
> @@ -0,0 +1,38 @@
> +/*
> + * nwfilter_dhcpsnoop.h: support DHCP snooping for a VM on an interface
> + *
> + * Copyright (C) 2010 IBM Corp.
> + * Copyright (C) 2010 David L Stevens
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
> + *
> + * Author: David L Stevens<dlstevens at us.ibm.com>
> + */
> +
> +#ifndef __NWFILTER_DHCPSNOOP_H
> +#define __NWFILTER_DHCPSNOOP_H
> +
> +int virNWFilterDHCPSnoopInit(void);
> +int virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
> +                            const char *ifname,
> +                            int ifindex,
> +                            const char *linkdev,
> +                            enum virDomainNetType nettype,
> +                            const unsigned char *macaddr,
> +                            const char *filtername,
> +                            virNWFilterHashTablePtr filterparams,
> +                            virNWFilterDriverStatePtr driver);
> +void virNWFilterDHCPSnoopEnd(const char *ifname);
> +#endif /* __NWFILTER_DHCPSNOOP_H */
> diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
> index a735059..b877d9d 100644
> --- a/src/nwfilter/nwfilter_driver.c
> +++ b/src/nwfilter/nwfilter_driver.c
> @@ -39,6 +39,7 @@
>   #include "nwfilter_gentech_driver.h"
>   #include "configmake.h"
>
> +#include "nwfilter_dhcpsnoop.h"
>   #include "nwfilter_learnipaddr.h"
>
>   #define VIR_FROM_THIS VIR_FROM_NWFILTER
> @@ -66,6 +67,8 @@ static int
>   nwfilterDriverStartup(int privileged) {
>       char *base = NULL;
>
> +    if (virNWFilterDHCPSnoopInit()<  0)
> +        return -1;
>       if (virNWFilterLearnInit()<  0)
>           return -1;
>
> @@ -127,6 +130,7 @@ alloc_err_exit:
>
>   conf_init_err:
>       virNWFilterTechDriversShutdown();
> +    virNWFilterDHCPSnoopEnd(0);
>       virNWFilterLearnShutdown();
>
>       return -1;
> @@ -149,6 +153,7 @@ nwfilterDriverReload(void) {
>       conn = virConnectOpen("qemu:///system");
>
>       if (conn) {
> +        virNWFilterDHCPSnoopEnd(0);
>           /* shut down all threads -- they will be restarted if necessary */
>           virNWFilterLearnThreadsTerminate(true);
>
> @@ -201,6 +206,7 @@ nwfilterDriverShutdown(void) {
>
>       virNWFilterConfLayerShutdown();
>       virNWFilterTechDriversShutdown();
> +    virNWFilterDHCPSnoopEnd(0);
>       virNWFilterLearnShutdown();
>
>       nwfilterDriverLock(driverState);
> diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
> index 7891983..2b10f9b 100644
> --- a/src/nwfilter/nwfilter_gentech_driver.c
> +++ b/src/nwfilter/nwfilter_gentech_driver.c
> @@ -33,6 +33,7 @@
>   #include "virterror_internal.h"
>   #include "nwfilter_gentech_driver.h"
>   #include "nwfilter_ebiptables_driver.h"
> +#include "nwfilter_dhcpsnoop.h"
>   #include "nwfilter_learnipaddr.h"
>
>
> @@ -42,6 +43,8 @@
>   #define NWFILTER_STD_VAR_MAC "MAC"
>   #define NWFILTER_STD_VAR_IP  "IP"
>
> +#define NWFILTER_DFLT_LEARN  "any"
> +
>   static int _virNWFilterTeardownFilter(const char *ifname);
>
>
> @@ -638,6 +641,8 @@ virNWFilterInstantiate(virConnectPtr conn,
>       void **ptrs = NULL;
>       int instantiate = 1;
>       char *buf;
> +    const char *learning;
> +    bool reportIP = false;
>
>       virNWFilterHashTablePtr missing_vars = virNWFilterHashTableCreate(0);
>       if (!missing_vars) {
> @@ -655,22 +660,42 @@ virNWFilterInstantiate(virConnectPtr conn,
>       if (rc)
>           goto err_exit;
>
> +    learning = virHashLookup(vars->hashTable, "ip_learning");
> +    if (learning == NULL)
> +        learning = NWFILTER_DFLT_LEARN;
> +
>       if (virHashSize(missing_vars->hashTable) == 1) {
>           if (virHashLookup(missing_vars->hashTable,
>                             NWFILTER_STD_VAR_IP) != NULL) {
> -            if (virNWFilterLookupLearnReq(ifindex) == NULL) {
> -                rc = virNWFilterLearnIPAddress(techdriver,
> -                                               ifname,
> -                                               ifindex,
> -                                               linkdev,
> -                                               nettype, macaddr,
> -                                               filter->name,
> -                                               vars, driver,
> -                                               DETECT_DHCP|DETECT_STATIC);
> +            if (c_strcasecmp(learning, "none") == 0) {        /* no learning */
> +                reportIP = true;
> +                goto err_unresolvable_vars;
>               }
> -            goto err_exit;
> -        }
> -        goto err_unresolvable_vars;
> +            if (c_strcasecmp(learning, "dhcp") == 0) {
> +                rc = virNWFilterDHCPSnoopReq(techdriver, ifname, ifindex,
> +                                             linkdev, nettype, macaddr,
> +                                             filter->name, vars, driver);
> +                goto err_exit;
> +            } else if (c_strcasecmp(learning, "any") == 0) {
> +                if (virNWFilterLookupLearnReq(ifindex) == NULL) {
> +                    rc = virNWFilterLearnIPAddress(techdriver,
> +                                                   ifname,
> +                                                   ifindex,
> +                                                   linkdev,
> +                                                   nettype, macaddr,
> +                                                   filter->name,
> +                                                   vars, driver,
> +                                                   DETECT_DHCP|DETECT_STATIC);
> +                }
> +                goto err_exit;
> +            } else {
> +                rc = 1;
> +                virNWFilterReportError(VIR_ERR_PARSE_FAILED, _("filter '%s' "
> +                                       "learning value '%s' invalid."),
> +                                       filter->name, learning);
> +            }
> +        } else
> +             goto err_unresolvable_vars;
>       } else if (virHashSize(missing_vars->hashTable)>  1) {
>           goto err_unresolvable_vars;
>       } else if (!forceWithPendingReq&&
> @@ -738,7 +763,7 @@ err_exit:
>
>   err_unresolvable_vars:
>
> -    buf = virNWFilterPrintVars(missing_vars->hashTable, ", ", false, false);
> +    buf = virNWFilterPrintVars(missing_vars->hashTable, ", ", false, reportIP);
>       if (buf) {
>           virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
>                      _("Cannot instantiate filter due to unresolvable "
> @@ -1070,6 +1095,8 @@ _virNWFilterTeardownFilter(const char *ifname)
>           return 1;
>       }
>
> +    virNWFilterDHCPSnoopEnd(ifname);
> +
>       virNWFilterTerminateLearnReq(ifname);
>
>       if (virNWFilterLockIface(ifname))
I have tried it now and ended up with this leasefile:

cat run/libvirt/network/nwfilter.leases
1321458854 vnet0 9.59.241.232 9.59.241.10
1321458915 vnet0 9.59.241.232 9.59.241.10

For some reason two entries ended up in the leasefile for the same IP 
address.

Any idea why 'kill -SIGHUP `pidof libvirtd`' doesn't restore the 
filtering rules? It cleans the ebtables tables but then nothing gets 
rebuilt. When restarting libvirtd it works as expected.

     Stefan





More information about the libvir-list mailing list