[B.A.T.M.A.N.] [PATCH] batman-adv: drop QinQ claim frames in bridge loop avoidance

[B.A.T.M.A.N.] [PATCH] batman-adv: drop QinQ claim frames in bridge loop avoidance

[Simon Wunderlich]

From: Simon Wunderlich <simon@open-mesh.com>

Since bridge loop avoidance only supports untagged or simple 802.1q
tagged VLAN claim frames, claim frames with stacked VLAN headers (QinQ)
should be detected and dropped. Transporting the over the mesh may cause
problems on the receivers, or create bogus entries in the local tt
tables.

Reported-by: Antonio Quartulli <antonio@open-mesh.com>
Signed-off-by: Simon Wunderlich <simon@open-mesh.com>
---
 bridge_loop_avoidance.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c
index 6f0d9ec..75274f7 100644
--- a/bridge_loop_avoidance.c
+++ b/bridge_loop_avoidance.c
@@ -851,6 +851,75 @@ static int batadv_check_claim_group(struct batadv_priv *bat_priv,
 	return 2;
 }
 
+/**
+ * batadv_is_encapsulated_claim
+ * @bat_priv: the bat priv with all the soft interface information
+ * @skb: skb to be checked
+ *
+ * Check if this frame is a claim frame encapsulated in at least
+ * two layers of VLANs.
+ *
+ * returns 1 if this is an encapsulated claim frame, 0 otherwise.
+ */
+static int batadv_is_encapsulated_claim(struct batadv_priv *bat_priv,
+					struct sk_buff *skb)
+{
+	struct batadv_bla_claim_dst *bla_dst, *bla_dst_own;
+	struct vlan_hdr vhdr, *vhdr_ptr;
+	uint8_t *hw_src, *hw_dst;
+	struct ethhdr *ethhdr;
+	struct arphdr *arphdr;
+	int headlen;
+
+	/* Traverse the VLAN/Ethertypes until an ARP header is found.
+	 *
+	 * At this point it is known that the first two protocols
+	 * are VLAN headers, so start checking at the encapsulated protocol
+	 * of the second header.
+	 */
+	headlen = VLAN_ETH_HLEN;
+	do {
+		vhdr_ptr = skb_header_pointer(skb, headlen, VLAN_HLEN, &vhdr);
+		if (!vhdr_ptr)
+			return 0;
+
+		headlen += VLAN_HLEN;
+	} while (vhdr_ptr->h_vlan_encapsulated_proto == htons(ETH_P_8021Q));
+
+	if (vhdr_ptr->h_vlan_encapsulated_proto != htons(ETH_P_ARP))
+		return 0;
+
+	if (unlikely(!pskb_may_pull(skb, headlen + arp_hdr_len(skb->dev))))
+		return 0;
+
+	/* pskb_may_pull() may have modified the pointers, get ethhdr again */
+	ethhdr = eth_hdr(skb);
+	arphdr = (struct arphdr *)((uint8_t *)ethhdr + headlen);
+
+	/* Check whether the ARP frame carries a valid IP information */
+	if (arphdr->ar_hrd != htons(ARPHRD_ETHER))
+		return 0;
+	if (arphdr->ar_pro != htons(ETH_P_IP))
+		return 0;
+	if (arphdr->ar_hln != ETH_ALEN)
+		return 0;
+	if (arphdr->ar_pln != 4)
+		return 0;
+
+	hw_src = (uint8_t *)arphdr + sizeof(struct arphdr);
+	hw_dst = hw_src + ETH_ALEN + 4;
+	bla_dst = (struct batadv_bla_claim_dst *)hw_dst;
+	bla_dst_own = &bat_priv->bla.claim_dest;
+
+	/* check if it is a claim frame in general */
+	if (memcmp(bla_dst->magic, bla_dst_own->magic,
+		   sizeof(bla_dst->magic)) != 0)
+		return 0;
+
+	batadv_dbg(BATADV_DBG_BLA, bat_priv, "Dropping encapsulated claim frame\n");
+
+	return 1;
+}
 
 /**
  * batadv_bla_process_claim
@@ -885,6 +954,21 @@ static int batadv_bla_process_claim(struct batadv_priv *bat_priv,
 		vhdr = vlan_eth_hdr(skb);
 		proto = vhdr->h_vlan_encapsulated_proto;
 		headlen += VLAN_HLEN;
+
+		if (proto == htons(ETH_P_8021Q)) {
+			/* check if there is a claim frame encapsulated
+			 * deeper in (QinQ) and drop that, as this is
+			 * not supported by BLA but should also not be
+			 * sent via the mesh.
+			 *
+			 * if its not, let the frame pass without further
+			 * checks, as it is not a claim frame anyway.
+			 */
+			if (batadv_is_encapsulated_claim(bat_priv, skb))
+				return 1;
+			else
+				return 0;
+		}
 	}
 
 	if (proto != htons(ETH_P_ARP))
-- 
1.9.1

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: drop QinQ claim frames in bridge loop avoidance

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 4299 bytes --]

Hi Simon,

On 11/06/14 20:43, Simon Wunderlich wrote:
> From: Simon Wunderlich <simon@open-mesh.com>
> 
> Since bridge loop avoidance only supports untagged or simple 802.1q
> tagged VLAN claim frames, claim frames with stacked VLAN headers (QinQ)
> should be detected and dropped. Transporting the over the mesh may cause
> problems on the receivers, or create bogus entries in the local tt
> tables.
> 
> Reported-by: Antonio Quartulli <antonio@open-mesh.com>
> Signed-off-by: Simon Wunderlich <simon@open-mesh.com>
> ---
>  bridge_loop_avoidance.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
> 
> diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c
> index 6f0d9ec..75274f7 100644
> --- a/bridge_loop_avoidance.c
> +++ b/bridge_loop_avoidance.c
> @@ -851,6 +851,75 @@ static int batadv_check_claim_group(struct batadv_priv *bat_priv,
>  	return 2;
>  }
>  
> +/**
> + * batadv_is_encapsulated_claim

not even a 4 words useless words for our short description? :)

> + * @bat_priv: the bat priv with all the soft interface information
> + * @skb: skb to be checked
> + *
> + * Check if this frame is a claim frame encapsulated in at least
> + * two layers of VLANs.
> + *
> + * returns 1 if this is an encapsulated claim frame, 0 otherwise.

Start with Capital letter here.

> + */
> +static int batadv_is_encapsulated_claim(struct batadv_priv *bat_priv,
> +					struct sk_buff *skb)
> +{
> +	struct batadv_bla_claim_dst *bla_dst, *bla_dst_own;
> +	struct vlan_hdr vhdr, *vhdr_ptr;
> +	uint8_t *hw_src, *hw_dst;
> +	struct ethhdr *ethhdr;
> +	struct arphdr *arphdr;
> +	int headlen;
> +
> +	/* Traverse the VLAN/Ethertypes until an ARP header is found.
> +	 *
> +	 * At this point it is known that the first two protocols
> +	 * are VLAN headers, so start checking at the encapsulated protocol
> +	 * of the second header.
> +	 */
> +	headlen = VLAN_ETH_HLEN;
> +	do {
> +		vhdr_ptr = skb_header_pointer(skb, headlen, VLAN_HLEN, &vhdr);
> +		if (!vhdr_ptr)
> +			return 0;
> +
> +		headlen += VLAN_HLEN;
> +	} while (vhdr_ptr->h_vlan_encapsulated_proto == htons(ETH_P_8021Q));
> +
> +	if (vhdr_ptr->h_vlan_encapsulated_proto != htons(ETH_P_ARP))
> +		return 0;
> +
> +	if (unlikely(!pskb_may_pull(skb, headlen + arp_hdr_len(skb->dev))))
> +		return 0;
> +
> +	/* pskb_may_pull() may have modified the pointers, get ethhdr again */
> +	ethhdr = eth_hdr(skb);
> +	arphdr = (struct arphdr *)((uint8_t *)ethhdr + headlen);
> +
> +	/* Check whether the ARP frame carries a valid IP information */
> +	if (arphdr->ar_hrd != htons(ARPHRD_ETHER))
> +		return 0;
> +	if (arphdr->ar_pro != htons(ETH_P_IP))
> +		return 0;
> +	if (arphdr->ar_hln != ETH_ALEN)
> +		return 0;
> +	if (arphdr->ar_pln != 4)
> +		return 0;
> +
> +	hw_src = (uint8_t *)arphdr + sizeof(struct arphdr);
> +	hw_dst = hw_src + ETH_ALEN + 4;
> +	bla_dst = (struct batadv_bla_claim_dst *)hw_dst;
> +	bla_dst_own = &bat_priv->bla.claim_dest;
> +
> +	/* check if it is a claim frame in general */
> +	if (memcmp(bla_dst->magic, bla_dst_own->magic,
> +		   sizeof(bla_dst->magic)) != 0)
> +		return 0;
> +
> +	batadv_dbg(BATADV_DBG_BLA, bat_priv, "Dropping encapsulated claim frame\n");

this function is not dropping anything. I'd rather move this message
where the real drop decision is made (e.g. in the chunk below).

> +
> +	return 1;
> +}
>  
>  /**
>   * batadv_bla_process_claim
> @@ -885,6 +954,21 @@ static int batadv_bla_process_claim(struct batadv_priv *bat_priv,
>  		vhdr = vlan_eth_hdr(skb);
>  		proto = vhdr->h_vlan_encapsulated_proto;
>  		headlen += VLAN_HLEN;
> +
> +		if (proto == htons(ETH_P_8021Q)) {
> +			/* check if there is a claim frame encapsulated
> +			 * deeper in (QinQ) and drop that, as this is
> +			 * not supported by BLA but should also not be
> +			 * sent via the mesh.
> +			 *
> +			 * if its not, let the frame pass without further
> +			 * checks, as it is not a claim frame anyway.
> +			 */
> +			if (batadv_is_encapsulated_claim(bat_priv, skb))
> +				return 1;
> +			else
> +				return 0;
> +		}
>  	}
>  
>  	if (proto != htons(ETH_P_ARP))



The rest looks good thanks!

-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]