[ovs-dev] [PATCH] Add Nicira extension to OpenFlow for dropping spoofed ARP packets.

Reid Price reid at nicira.com
Thu Aug 12 23:09:20 UTC 2010


Also Paul, I think this takes care of one of the requirements for VMs to
send traffic on the wrong lswitch in
3273<http://redmine.nicira.com/issues/3273>,
so we don't have to worry about that anymore =)

On Thu, Aug 12, 2010 at 2:07 PM, Martin Casado <casado at nicira.com> wrote:

> This is great Ben, and sorely needed.  Thanks for getting that patch in.
> ovs security ++.
>
>
>
>  "ARP spoofing" is when a host claims an incorrect association between an
>> IP address and a MAC address for deceptive purposes.  OpenFlow by itself
>> can prevent a host from sending out ARP replies from an incorrect MAC
>> address in the Ethernet L2 header, but it cannot control the MAC addresses
>> inside the ARP L3 packet.  This commit adds a new action that can be used
>> to drop these spoofed packets.
>>
>> CC: Paul Ingram <paul at nicira.com>
>> ---
>> I'm sending this out partly as an RFC, to check with Paul that this
>> solves the problem that he has, and to make sure that this type of
>> solution is OK with the other developers.
>>
>> I have not tested this code yet.
>>
>>  datapath/actions.c                      |   27
>> +++++++++++++++++++++++++++
>>  datapath/flow.c                         |   15 ---------------
>>  datapath/flow.h                         |   16 ++++++++++++++++
>>  include/openflow/nicira-ext.h           |   13 ++++++++++++-
>>  include/openvswitch/datapath-protocol.h |    3 ++-
>>  ofproto/ofproto.c                       |    6 ++++++
>>  6 files changed, 63 insertions(+), 17 deletions(-)
>>
>> diff --git a/datapath/actions.c b/datapath/actions.c
>> index a6771b6..81e1c84 100644
>> --- a/datapath/actions.c
>> +++ b/datapath/actions.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/tcp.h>
>>  #include <linux/udp.h>
>>  #include <linux/in6.h>
>> +#include <linux/if_arp.h>
>>  #include <linux/if_vlan.h>
>>  #include <net/inet_ecn.h>
>>  #include <net/ip.h>
>> @@ -318,6 +319,26 @@ static struct sk_buff *set_tp_port(struct sk_buff
>> *skb,
>>        return skb;
>>  }
>>  +/* Returns true if 'skb' is an Ethernet+IPv4 ARP packet whose inner and
>> outer
>> + * Ethernet address differ. */
>> +static bool is_spoofed_arp(struct sk_buff *skb, const struct odp_flow_key
>> *key)
>> +{
>> +       struct arp_eth_header *arp;
>> +
>> +       if (key->dl_type != htons(ETH_P_ARP))
>> +               return false;
>> +
>> +       if (skb_network_offset(skb) + sizeof(struct arp_eth_header) >
>> skb->len)
>> +               return false;
>> +
>> +       arp = (struct arp_eth_header *)skb_network_header(skb);
>> +       return (arp->ar_hrd == htons(ARPHRD_ETHER) &&
>> +               arp->ar_pro == htons(ETH_P_IP) &&
>> +               arp->ar_hln == ETH_ALEN &&
>> +               arp->ar_pln == 4 &&
>> +               compare_ether_addr(arp->ar_sha, eth_hdr(skb)->h_source));
>> +}
>> +
>>  static void do_output(struct datapath *dp, struct sk_buff *skb, int
>> out_port)
>>  {
>>        struct dp_port *p;
>> @@ -484,10 +505,16 @@ int execute_actions(struct datapath *dp, struct
>> sk_buff *skb,
>>                case ODPAT_POP_PRIORITY:
>>                        skb->priority = priority;
>>                        break;
>> +
>> +               case ODPAT_DROP_SPOOFED_ARP:
>> +                       if (is_spoofed_arp(skb, key))
>> +                               goto exit;
>> +                       break;
>>                }
>>                if (!skb)
>>                        return -ENOMEM;
>>        }
>> +exit:
>>        if (prev_port != -1)
>>                do_output(dp, skb, prev_port);
>>        else
>> diff --git a/datapath/flow.c b/datapath/flow.c
>> index f769b14..8074b20 100644
>> --- a/datapath/flow.c
>> +++ b/datapath/flow.c
>> @@ -34,21 +34,6 @@
>>  struct kmem_cache *flow_cache;
>>  static unsigned int hash_seed;
>>  -struct arp_eth_header
>> -{
>> -       __be16      ar_hrd;     /* format of hardware address   */
>> -       __be16      ar_pro;     /* format of protocol address   */
>> -       unsigned char   ar_hln; /* length of hardware address   */
>> -       unsigned char   ar_pln; /* length of protocol address   */
>> -       __be16      ar_op;      /* ARP opcode (command)     */
>> -
>> -       /* Ethernet+IPv4 specific members. */
>> -       unsigned char       ar_sha[ETH_ALEN];   /* sender hardware address
>>  */
>> -       unsigned char       ar_sip[4];          /* sender IP address
>>  */
>> -       unsigned char       ar_tha[ETH_ALEN];   /* target hardware address
>>  */
>> -       unsigned char       ar_tip[4];          /* target IP address
>>  */
>> -} __attribute__((packed));
>> -
>>  static inline int arphdr_ok(struct sk_buff *skb)
>>  {
>>        int nh_ofs = skb_network_offset(skb);
>> diff --git a/datapath/flow.h b/datapath/flow.h
>> index 534c7af..80a5b66 100644
>> --- a/datapath/flow.h
>> +++ b/datapath/flow.h
>> @@ -14,6 +14,7 @@
>>  #include <linux/types.h>
>>  #include <linux/rcupdate.h>
>>  #include <linux/gfp.h>
>> +#include <linux/if_ether.h>
>>  #include <linux/jiffies.h>
>>  #include <linux/time.h>
>>  @@ -42,6 +43,21 @@ struct sw_flow {
>>        u8 tcp_flags;           /* Union of seen TCP flags. */
>>  };
>>  +struct arp_eth_header
>> +{
>> +       __be16      ar_hrd;     /* format of hardware address   */
>> +       __be16      ar_pro;     /* format of protocol address   */
>> +       unsigned char   ar_hln; /* length of hardware address   */
>> +       unsigned char   ar_pln; /* length of protocol address   */
>> +       __be16      ar_op;      /* ARP opcode (command)     */
>> +
>> +       /* Ethernet+IPv4 specific members. */
>> +       unsigned char       ar_sha[ETH_ALEN];   /* sender hardware address
>>  */
>> +       unsigned char       ar_sip[4];          /* sender IP address
>>  */
>> +       unsigned char       ar_tha[ETH_ALEN];   /* target hardware address
>>  */
>> +       unsigned char       ar_tip[4];          /* target IP address
>>  */
>> +} __attribute__((packed));
>> +
>>  extern struct kmem_cache *flow_cache;
>>  struct sw_flow_actions *flow_actions_alloc(size_t n_actions);
>> diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
>> index e6f34ba..f2c5b73 100644
>> --- a/include/openflow/nicira-ext.h
>> +++ b/include/openflow/nicira-ext.h
>> @@ -129,7 +129,18 @@ enum nx_action_subtype {
>>      */
>>     NXAST_RESUBMIT,
>>  -    NXAST_SET_TUNNEL                /* Set encapsulating tunnel ID. */
>> +    /* Set encapsulating tunnel ID. */
>> +    NXAST_SET_TUNNEL,
>> +
>> +    /* Stops processing further actions, if the packet being processed is
>> an
>> +     * Ethernet+IPv4 ARP packet for which the source Ethernet address
>> inside
>> +     * the ARP packet differs from the source Ethernet address in the
>> Ethernet
>> +     * header.
>> +     *
>> +     * This is useful because OpenFlow does not provide a way to match on
>> the
>> +     * Ethernet addresses inside ARP packets, so there is no other way to
>> drop
>> +     * spoofed ARPs other than sending every packet up to the controller.
>> */
>> +    NXAST_DROP_SPOOFED_ARP
>>  };
>>  /* Action structure for NXAST_RESUBMIT. */
>> diff --git a/include/openvswitch/datapath-protocol.h
>> b/include/openvswitch/datapath-protocol.h
>> index c3ec4dc..839c484 100644
>> --- a/include/openvswitch/datapath-protocol.h
>> +++ b/include/openvswitch/datapath-protocol.h
>> @@ -281,7 +281,8 @@ struct odp_flowvec {
>>  #define ODPAT_SET_TUNNEL        13   /* Set the encapsulating tunnel ID.
>> */
>>  #define ODPAT_SET_PRIORITY      14   /* Set skb->priority. */
>>  #define ODPAT_POP_PRIORITY      15   /* Restore original skb->priority.
>> */
>> -#define ODPAT_N_ACTIONS         16
>> +#define ODPAT_DROP_SPOOFED_ARP  16   /* Drop ARPs with spoofed source
>> MAC. */
>> +#define ODPAT_N_ACTIONS         17
>>  struct odp_action_output {
>>     uint16_t type;              /* ODPAT_OUTPUT. */
>> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
>> index a466c9c..c0d4acc 100644
>> --- a/ofproto/ofproto.c
>> +++ b/ofproto/ofproto.c
>> @@ -2635,6 +2635,12 @@ xlate_nicira_action(struct action_xlate_ctx *ctx,
>>         ctx->flow.tun_id = oa->tunnel.tun_id = nast->tun_id;
>>         break;
>>  +    case NXAST_DROP_SPOOFED_ARP:
>> +        if (ctx->flow.dl_type == htons(ETH_TYPE_ARP)) {
>> +            odp_actions_add(ctx->out, ODPAT_DROP_SPOOFED_ARP);
>> +        }
>> +        break;
>> +
>>     /* If you add a new action here that modifies flow data, don't forget
>> to
>>      * update the flow key in ctx->flow at the same time. */
>>
>>
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20100812/417017b1/attachment-0003.html>


More information about the dev mailing list