Patchwork ipv6: add anti-spoofing checks for 6to4 and 6rd

login
register
mail settings
Submitter Hannes Frederic Sowa
Date Jan. 17, 2013, 3:32 a.m.
Message ID <20130117033258.GA23782@order.stressinduktion.org>
Download mbox | patch
Permalink /patch/213132/
State Superseded
Delegated to: David Miller
Headers show

Comments

Hannes Frederic Sowa - Jan. 17, 2013, 3:32 a.m.
This patch adds anti-spoofing checks in sit.c as specified in RFC3964
section 5.2 for 6to4 and RFC5969 section 12 for 6rd. I left out the
checks which could easily be implemented with netfilter.

Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/sit.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)
Hannes Frederic Sowa - Jan. 17, 2013, 1:27 p.m.
On Thu, Jan 17, 2013 at 04:32:58AM +0100, Hannes Frederic Sowa wrote:
> +static int sit_chksrc(struct ip_tunnel *tunnel, const __be32 *addr,
> +		const struct in6_addr *addr6)
> +{
> +#ifdef CONFIG_IPV6_SIT_6RD
> +	if (ipv6_prefix_equal(addr6, &tunnel->ip6rd.prefix,
> +				tunnel->ip6rd.prefixlen) &&
> +		memcmp(addr, &addr6->s6_addr16[1], 4))
> +		return 0;

This is wrong. David, please drop my patch, sorry. I will respin another one.
In 6rd setups one can, of course, have longer prefixlens than 16.

Thanks!

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
YOSHIFUJI Hideaki / 吉藤英明 - Jan. 17, 2013, 3:44 p.m.
Hannes Frederic Sowa wrote:
> This patch adds anti-spoofing checks in sit.c as specified in RFC3964
> section 5.2 for 6to4 and RFC5969 section 12 for 6rd. I left out the
> checks which could easily be implemented with netfilter.
> 
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  net/ipv6/sit.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
> index cfba99b..2b4c15a 100644
> --- a/net/ipv6/sit.c
> +++ b/net/ipv6/sit.c
> @@ -590,6 +590,22 @@ out:
>  	return err;
>  }
>  
> +static int sit_chksrc(struct ip_tunnel *tunnel, const __be32 *addr,
> +		const struct in6_addr *addr6)
> +{
> +#ifdef CONFIG_IPV6_SIT_6RD
> +	if (ipv6_prefix_equal(addr6, &tunnel->ip6rd.prefix,
> +				tunnel->ip6rd.prefixlen) &&
> +		memcmp(addr, &addr6->s6_addr16[1], 4))
> +		return 0;
> +#else
> +	if (addr6->s6_addr16[0] == htons(0x2002) &&
> +		memcmp(addr, &addr6->s6_addr16[1], 4))
> +		return 0;
> +#endif
> +	return 1;
>

It seems wrong.  Check should be done for
- inner source prefix
- embedded source with relay_prefix.
- inner destination prefix.

Note: embedded destination is not being checked.

--yoshfuji
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hannes Frederic Sowa - Jan. 17, 2013, 4:17 p.m.
On Fri, Jan 18, 2013 at 12:44:11AM +0900, YOSHIFUJI Hideaki wrote:
> Hannes Frederic Sowa wrote:
> > This patch adds anti-spoofing checks in sit.c as specified in RFC3964
> > section 5.2 for 6to4 and RFC5969 section 12 for 6rd. I left out the
> > checks which could easily be implemented with netfilter.
> > 
> > Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > ---
> >  net/ipv6/sit.c | 27 +++++++++++++++++++++++++--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
> > index cfba99b..2b4c15a 100644
> > --- a/net/ipv6/sit.c
> > +++ b/net/ipv6/sit.c
> > @@ -590,6 +590,22 @@ out:
> >  	return err;
> >  }
> >  
> > +static int sit_chksrc(struct ip_tunnel *tunnel, const __be32 *addr,
> > +		const struct in6_addr *addr6)
> > +{
> > +#ifdef CONFIG_IPV6_SIT_6RD
> > +	if (ipv6_prefix_equal(addr6, &tunnel->ip6rd.prefix,
> > +				tunnel->ip6rd.prefixlen) &&
> > +		memcmp(addr, &addr6->s6_addr16[1], 4))
> > +		return 0;
> > +#else
> > +	if (addr6->s6_addr16[0] == htons(0x2002) &&
> > +		memcmp(addr, &addr6->s6_addr16[1], 4))
> > +		return 0;
> > +#endif
> > +	return 1;
> >
> 
> It seems wrong.  Check should be done for
> - inner source prefix

I intentionally skipped this check because it could be easily checked
with netfilter (after decapsulation) and I am a bit afraid breaking
already working setups with non-standard prefixes. Do you think I should
add this check anyway?

> - embedded source with relay_prefix.

I'll use try_6rd to extract the ipv4 address and check it against the
outer address. I will check this later if I have access to my test setup.

> - inner destination prefix.
>
> Note: embedded destination is not being checked.

Also left these checks out because of the same reasons I stated above. Should
they be added?

Thanks for the review!

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index cfba99b..2b4c15a 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -590,6 +590,22 @@  out:
 	return err;
 }
 
+static int sit_chksrc(struct ip_tunnel *tunnel, const __be32 *addr,
+		const struct in6_addr *addr6)
+{
+#ifdef CONFIG_IPV6_SIT_6RD
+	if (ipv6_prefix_equal(addr6, &tunnel->ip6rd.prefix,
+				tunnel->ip6rd.prefixlen) &&
+		memcmp(addr, &addr6->s6_addr16[1], 4))
+		return 0;
+#else
+	if (addr6->s6_addr16[0] == htons(0x2002) &&
+		memcmp(addr, &addr6->s6_addr16[1], 4))
+		return 0;
+#endif
+	return 1;
+}
+
 static int ipip6_rcv(struct sk_buff *skb)
 {
 	const struct iphdr *iph;
@@ -613,8 +629,15 @@  static int ipip6_rcv(struct sk_buff *skb)
 		skb->protocol = htons(ETH_P_IPV6);
 		skb->pkt_type = PACKET_HOST;
 
-		if ((tunnel->dev->priv_flags & IFF_ISATAP) &&
-		    !isatap_chksrc(skb, iph, tunnel)) {
+		if (tunnel->dev->priv_flags & IFF_ISATAP) {
+			if (!isatap_chksrc(skb, iph, tunnel)) {
+				tunnel->dev->stats.rx_errors++;
+				goto out;
+			}
+		} else if (!sit_chksrc(tunnel, &iph->saddr,
+					&ipv6_hdr(skb)->saddr) ||
+			!sit_chksrc(tunnel, &iph->daddr,
+				&ipv6_hdr(skb)->daddr)) {
 			tunnel->dev->stats.rx_errors++;
 			goto out;
 		}