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

Justin Pettit jpettit at nicira.com
Fri Aug 13 16:47:27 UTC 2010


The implementation looks good to me.

--Justin


On Aug 12, 2010, at 2:04 PM, Ben Pfaff wrote:

> "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. */
> 
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org





More information about the dev mailing list