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

Martin Casado casado at nicira.com
Thu Aug 12 21:07:58 UTC 2010


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. */
>  
>   





More information about the dev mailing list