[PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[Eric Dumazet]

IPv6 fragment identification generation is way beyond what we use for
IPv4 : It uses a single generator. Its not scalable and allows DOS
attacks.

Now inetpeer is IPv6 aware, we can use it to provide a more secure and
scalable frag ident generator (per destination, instead of system wide)

This patch :
1) defines a new secure_ipv6_id() helper
2) extends inet_getid() to provide 32bit results
3) extends ipv6_select_ident() with a new dest parameter

Reported-by: Fernando Gont <fernando@gont.com.ar>
CC: Matt Mackall <mpm@selenic.com>
CC: Eugene Teo <eugeneteo@kernel.sg>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/char/random.c  |   15 +++++++++++++++
 include/linux/random.h |    1 +
 include/net/inetpeer.h |   13 ++++++++++---
 include/net/ipv6.h     |   12 +-----------
 net/ipv4/inetpeer.c    |    7 +++++--
 net/ipv6/ip6_output.c  |   36 +++++++++++++++++++++++++++++++-----
 net/ipv6/udp.c         |    2 +-
 7 files changed, 64 insertions(+), 22 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index d4ddeba..7292819 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1523,6 +1523,21 @@ __u32 secure_ip_id(__be32 daddr)
 	return half_md4_transform(hash, keyptr->secret);
 }
 
+__u32 secure_ipv6_id(const __be32 daddr[4])
+{
+	const struct keydata *keyptr;
+	__u32 hash[4];
+
+	keyptr = get_keyptr();
+
+	hash[0] = (__force __u32)daddr[0];
+	hash[1] = (__force __u32)daddr[1];
+	hash[2] = (__force __u32)daddr[2];
+	hash[3] = (__force __u32)daddr[3];
+
+	return half_md4_transform(hash, keyptr->secret);
+}
+
 #ifdef CONFIG_INET
 
 __u32 secure_tcp_sequence_number(__be32 saddr, __be32 daddr,
diff --git a/include/linux/random.h b/include/linux/random.h
index fb7ab9d..ce29a04 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -58,6 +58,7 @@ extern void get_random_bytes(void *buf, int nbytes);
 void generate_random_uuid(unsigned char uuid_out[16]);
 
 extern __u32 secure_ip_id(__be32 daddr);
+extern __u32 secure_ipv6_id(const __be32 daddr[4]);
 extern u32 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport);
 extern u32 secure_ipv6_port_ephemeral(const __be32 *saddr, const __be32 *daddr,
 				      __be16 dport);
diff --git a/include/net/inetpeer.h b/include/net/inetpeer.h
index 39d1230..4233e6f 100644
--- a/include/net/inetpeer.h
+++ b/include/net/inetpeer.h
@@ -71,7 +71,7 @@ static inline bool inet_metrics_new(const struct inet_peer *p)
 }
 
 /* can be called with or without local BH being disabled */
-struct inet_peer	*inet_getpeer(struct inetpeer_addr *daddr, int create);
+struct inet_peer	*inet_getpeer(const struct inetpeer_addr *daddr, int create);
 
 static inline struct inet_peer *inet_getpeer_v4(__be32 v4daddr, int create)
 {
@@ -106,11 +106,18 @@ static inline void inet_peer_refcheck(const struct inet_peer *p)
 
 
 /* can be called with or without local BH being disabled */
-static inline __u16	inet_getid(struct inet_peer *p, int more)
+static inline int inet_getid(struct inet_peer *p, int more)
 {
+	int old, new;
 	more++;
 	inet_peer_refcheck(p);
-	return atomic_add_return(more, &p->ip_id_count) - more;
+	do {
+		old = atomic_read(&p->ip_id_count);
+		new = old + more;
+		if (!new)
+			new = 1;
+	} while (atomic_cmpxchg(&p->ip_id_count, old, new) != old);
+	return new;
 }
 
 #endif /* _NET_INETPEER_H */
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index c033ed0..3b5ac1f 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -463,17 +463,7 @@ static inline int ipv6_addr_diff(const struct in6_addr *a1, const struct in6_add
 	return __ipv6_addr_diff(a1, a2, sizeof(struct in6_addr));
 }
 
-static __inline__ void ipv6_select_ident(struct frag_hdr *fhdr)
-{
-	static u32 ipv6_fragmentation_id = 1;
-	static DEFINE_SPINLOCK(ip6_id_lock);
-
-	spin_lock_bh(&ip6_id_lock);
-	fhdr->identification = htonl(ipv6_fragmentation_id);
-	if (++ipv6_fragmentation_id == 0)
-		ipv6_fragmentation_id = 1;
-	spin_unlock_bh(&ip6_id_lock);
-}
+extern void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt);
 
 /*
  *	Prototypes exported by ipv6
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index 90c5f0d..e382138 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -391,7 +391,7 @@ static int inet_peer_gc(struct inet_peer_base *base,
 	return cnt;
 }
 
-struct inet_peer *inet_getpeer(struct inetpeer_addr *daddr, int create)
+struct inet_peer *inet_getpeer(const struct inetpeer_addr *daddr, int create)
 {
 	struct inet_peer __rcu **stack[PEER_MAXDEPTH], ***stackptr;
 	struct inet_peer_base *base = family_to_base(daddr->family);
@@ -436,7 +436,10 @@ relookup:
 		p->daddr = *daddr;
 		atomic_set(&p->refcnt, 1);
 		atomic_set(&p->rid, 0);
-		atomic_set(&p->ip_id_count, secure_ip_id(daddr->addr.a4));
+		atomic_set(&p->ip_id_count,
+				(daddr->family == AF_INET) ?
+					secure_ip_id(daddr->addr.a4) :
+					secure_ipv6_id(daddr->addr.a6));
 		p->tcp_ts_stamp = 0;
 		p->metrics[RTAX_LOCK-1] = INETPEER_METRICS_NEW;
 		p->rate_tokens = 0;
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 8db0e48..32e5339 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -596,6 +596,31 @@ int ip6_find_1stfragopt(struct sk_buff *skb, u8 **nexthdr)
 	return offset;
 }
 
+void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt)
+{
+	static atomic_t ipv6_fragmentation_id;
+	int old, new;
+
+	if (rt) {
+		struct inet_peer *peer;
+
+		if (!rt->rt6i_peer)
+			rt6_bind_peer(rt, 1);
+		peer = rt->rt6i_peer;
+		if (peer) {
+			fhdr->identification = htonl(inet_getid(peer, 0));
+			return;
+		}
+	}
+	do {
+		old = atomic_read(&ipv6_fragmentation_id);
+		new = old + 1;
+		if (!new)
+			new = 1;
+	} while (atomic_cmpxchg(&ipv6_fragmentation_id, old, new) != old);
+	fhdr->identification = htonl(new);
+}
+
 int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 {
 	struct sk_buff *frag;
@@ -680,7 +705,7 @@ int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 		skb_reset_network_header(skb);
 		memcpy(skb_network_header(skb), tmp_hdr, hlen);
 
-		ipv6_select_ident(fh);
+		ipv6_select_ident(fh, rt);
 		fh->nexthdr = nexthdr;
 		fh->reserved = 0;
 		fh->frag_off = htons(IP6_MF);
@@ -826,7 +851,7 @@ slow_path:
 		fh->nexthdr = nexthdr;
 		fh->reserved = 0;
 		if (!frag_id) {
-			ipv6_select_ident(fh);
+			ipv6_select_ident(fh, rt);
 			frag_id = fh->identification;
 		} else
 			fh->identification = frag_id;
@@ -1076,7 +1101,8 @@ static inline int ip6_ufo_append_data(struct sock *sk,
 			int getfrag(void *from, char *to, int offset, int len,
 			int odd, struct sk_buff *skb),
 			void *from, int length, int hh_len, int fragheaderlen,
-			int transhdrlen, int mtu,unsigned int flags)
+			int transhdrlen, int mtu,unsigned int flags,
+			struct rt6_info *rt)
 
 {
 	struct sk_buff *skb;
@@ -1120,7 +1146,7 @@ static inline int ip6_ufo_append_data(struct sock *sk,
 		skb_shinfo(skb)->gso_size = (mtu - fragheaderlen -
 					     sizeof(struct frag_hdr)) & ~7;
 		skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
-		ipv6_select_ident(&fhdr);
+		ipv6_select_ident(&fhdr, rt);
 		skb_shinfo(skb)->ip6_frag_id = fhdr.identification;
 		__skb_queue_tail(&sk->sk_write_queue, skb);
 
@@ -1286,7 +1312,7 @@ int ip6_append_data(struct sock *sk, int getfrag(void *from, char *to,
 
 			err = ip6_ufo_append_data(sk, getfrag, from, length,
 						  hh_len, fragheaderlen,
-						  transhdrlen, mtu, flags);
+						  transhdrlen, mtu, flags, rt);
 			if (err)
 				goto error;
 			return 0;
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 328985c..29213b5 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1359,7 +1359,7 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb, u32 features)
 	fptr = (struct frag_hdr *)(skb_network_header(skb) + unfrag_ip6hlen);
 	fptr->nexthdr = nexthdr;
 	fptr->reserved = 0;
-	ipv6_select_ident(fptr);
+	ipv6_select_ident(fptr, (struct rt6_info *)skb_dst(skb));
 
 	/* Fragment the skb. ipv6 header and the remaining fields of the
 	 * fragment header are updated in ipv6_gso_segment()

Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[Matt Mackall]

On Tue, 2011-07-19 at 22:47 +0200, Eric Dumazet wrote:
> IPv6 fragment identification generation is way beyond what we use for
> IPv4 : It uses a single generator. Its not scalable and allows DOS
> attacks.
> 
> Now inetpeer is IPv6 aware, we can use it to provide a more secure and
> scalable frag ident generator (per destination, instead of system wide)

This code really needs to get moved out of random.c and into net/. Other
than that, looks fine to me.

> This patch :
> 1) defines a new secure_ipv6_id() helper
> 2) extends inet_getid() to provide 32bit results
> 3) extends ipv6_select_ident() with a new dest parameter
> 
> Reported-by: Fernando Gont <fernando@gont.com.ar>
> CC: Matt Mackall <mpm@selenic.com>
> CC: Eugene Teo <eugeneteo@kernel.sg>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  drivers/char/random.c  |   15 +++++++++++++++
>  include/linux/random.h |    1 +
>  include/net/inetpeer.h |   13 ++++++++++---
>  include/net/ipv6.h     |   12 +-----------
>  net/ipv4/inetpeer.c    |    7 +++++--
>  net/ipv6/ip6_output.c  |   36 +++++++++++++++++++++++++++++++-----
>  net/ipv6/udp.c         |    2 +-
>  7 files changed, 64 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index d4ddeba..7292819 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -1523,6 +1523,21 @@ __u32 secure_ip_id(__be32 daddr)
>  	return half_md4_transform(hash, keyptr->secret);
>  }
>  
> +__u32 secure_ipv6_id(const __be32 daddr[4])
> +{
> +	const struct keydata *keyptr;
> +	__u32 hash[4];
> +
> +	keyptr = get_keyptr();
> +
> +	hash[0] = (__force __u32)daddr[0];
> +	hash[1] = (__force __u32)daddr[1];
> +	hash[2] = (__force __u32)daddr[2];
> +	hash[3] = (__force __u32)daddr[3];
> +
> +	return half_md4_transform(hash, keyptr->secret);
> +}
> +
>  #ifdef CONFIG_INET
>  
>  __u32 secure_tcp_sequence_number(__be32 saddr, __be32 daddr,
> diff --git a/include/linux/random.h b/include/linux/random.h
> index fb7ab9d..ce29a04 100644
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> @@ -58,6 +58,7 @@ extern void get_random_bytes(void *buf, int nbytes);
>  void generate_random_uuid(unsigned char uuid_out[16]);
>  
>  extern __u32 secure_ip_id(__be32 daddr);
> +extern __u32 secure_ipv6_id(const __be32 daddr[4]);
>  extern u32 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport);
>  extern u32 secure_ipv6_port_ephemeral(const __be32 *saddr, const __be32 *daddr,
>  				      __be16 dport);
> diff --git a/include/net/inetpeer.h b/include/net/inetpeer.h
> index 39d1230..4233e6f 100644
> --- a/include/net/inetpeer.h
> +++ b/include/net/inetpeer.h
> @@ -71,7 +71,7 @@ static inline bool inet_metrics_new(const struct inet_peer *p)
>  }
>  
>  /* can be called with or without local BH being disabled */
> -struct inet_peer	*inet_getpeer(struct inetpeer_addr *daddr, int create);
> +struct inet_peer	*inet_getpeer(const struct inetpeer_addr *daddr, int create);
>  
>  static inline struct inet_peer *inet_getpeer_v4(__be32 v4daddr, int create)
>  {
> @@ -106,11 +106,18 @@ static inline void inet_peer_refcheck(const struct inet_peer *p)
>  
> 
>  /* can be called with or without local BH being disabled */
> -static inline __u16	inet_getid(struct inet_peer *p, int more)
> +static inline int inet_getid(struct inet_peer *p, int more)
>  {
> +	int old, new;
>  	more++;
>  	inet_peer_refcheck(p);
> -	return atomic_add_return(more, &p->ip_id_count) - more;
> +	do {
> +		old = atomic_read(&p->ip_id_count);
> +		new = old + more;
> +		if (!new)
> +			new = 1;
> +	} while (atomic_cmpxchg(&p->ip_id_count, old, new) != old);
> +	return new;
>  }
>  
>  #endif /* _NET_INETPEER_H */
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index c033ed0..3b5ac1f 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -463,17 +463,7 @@ static inline int ipv6_addr_diff(const struct in6_addr *a1, const struct in6_add
>  	return __ipv6_addr_diff(a1, a2, sizeof(struct in6_addr));
>  }
>  
> -static __inline__ void ipv6_select_ident(struct frag_hdr *fhdr)
> -{
> -	static u32 ipv6_fragmentation_id = 1;
> -	static DEFINE_SPINLOCK(ip6_id_lock);
> -
> -	spin_lock_bh(&ip6_id_lock);
> -	fhdr->identification = htonl(ipv6_fragmentation_id);
> -	if (++ipv6_fragmentation_id == 0)
> -		ipv6_fragmentation_id = 1;
> -	spin_unlock_bh(&ip6_id_lock);
> -}
> +extern void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt);
>  
>  /*
>   *	Prototypes exported by ipv6
> diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
> index 90c5f0d..e382138 100644
> --- a/net/ipv4/inetpeer.c
> +++ b/net/ipv4/inetpeer.c
> @@ -391,7 +391,7 @@ static int inet_peer_gc(struct inet_peer_base *base,
>  	return cnt;
>  }
>  
> -struct inet_peer *inet_getpeer(struct inetpeer_addr *daddr, int create)
> +struct inet_peer *inet_getpeer(const struct inetpeer_addr *daddr, int create)
>  {
>  	struct inet_peer __rcu **stack[PEER_MAXDEPTH], ***stackptr;
>  	struct inet_peer_base *base = family_to_base(daddr->family);
> @@ -436,7 +436,10 @@ relookup:
>  		p->daddr = *daddr;
>  		atomic_set(&p->refcnt, 1);
>  		atomic_set(&p->rid, 0);
> -		atomic_set(&p->ip_id_count, secure_ip_id(daddr->addr.a4));
> +		atomic_set(&p->ip_id_count,
> +				(daddr->family == AF_INET) ?
> +					secure_ip_id(daddr->addr.a4) :
> +					secure_ipv6_id(daddr->addr.a6));
>  		p->tcp_ts_stamp = 0;
>  		p->metrics[RTAX_LOCK-1] = INETPEER_METRICS_NEW;
>  		p->rate_tokens = 0;
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 8db0e48..32e5339 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -596,6 +596,31 @@ int ip6_find_1stfragopt(struct sk_buff *skb, u8 **nexthdr)
>  	return offset;
>  }
>  
> +void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt)
> +{
> +	static atomic_t ipv6_fragmentation_id;
> +	int old, new;
> +
> +	if (rt) {
> +		struct inet_peer *peer;
> +
> +		if (!rt->rt6i_peer)
> +			rt6_bind_peer(rt, 1);
> +		peer = rt->rt6i_peer;
> +		if (peer) {
> +			fhdr->identification = htonl(inet_getid(peer, 0));
> +			return;
> +		}
> +	}
> +	do {
> +		old = atomic_read(&ipv6_fragmentation_id);
> +		new = old + 1;
> +		if (!new)
> +			new = 1;
> +	} while (atomic_cmpxchg(&ipv6_fragmentation_id, old, new) != old);
> +	fhdr->identification = htonl(new);
> +}
> +
>  int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
>  {
>  	struct sk_buff *frag;
> @@ -680,7 +705,7 @@ int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
>  		skb_reset_network_header(skb);
>  		memcpy(skb_network_header(skb), tmp_hdr, hlen);
>  
> -		ipv6_select_ident(fh);
> +		ipv6_select_ident(fh, rt);
>  		fh->nexthdr = nexthdr;
>  		fh->reserved = 0;
>  		fh->frag_off = htons(IP6_MF);
> @@ -826,7 +851,7 @@ slow_path:
>  		fh->nexthdr = nexthdr;
>  		fh->reserved = 0;
>  		if (!frag_id) {
> -			ipv6_select_ident(fh);
> +			ipv6_select_ident(fh, rt);
>  			frag_id = fh->identification;
>  		} else
>  			fh->identification = frag_id;
> @@ -1076,7 +1101,8 @@ static inline int ip6_ufo_append_data(struct sock *sk,
>  			int getfrag(void *from, char *to, int offset, int len,
>  			int odd, struct sk_buff *skb),
>  			void *from, int length, int hh_len, int fragheaderlen,
> -			int transhdrlen, int mtu,unsigned int flags)
> +			int transhdrlen, int mtu,unsigned int flags,
> +			struct rt6_info *rt)
>  
>  {
>  	struct sk_buff *skb;
> @@ -1120,7 +1146,7 @@ static inline int ip6_ufo_append_data(struct sock *sk,
>  		skb_shinfo(skb)->gso_size = (mtu - fragheaderlen -
>  					     sizeof(struct frag_hdr)) & ~7;
>  		skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
> -		ipv6_select_ident(&fhdr);
> +		ipv6_select_ident(&fhdr, rt);
>  		skb_shinfo(skb)->ip6_frag_id = fhdr.identification;
>  		__skb_queue_tail(&sk->sk_write_queue, skb);
>  
> @@ -1286,7 +1312,7 @@ int ip6_append_data(struct sock *sk, int getfrag(void *from, char *to,
>  
>  			err = ip6_ufo_append_data(sk, getfrag, from, length,
>  						  hh_len, fragheaderlen,
> -						  transhdrlen, mtu, flags);
> +						  transhdrlen, mtu, flags, rt);
>  			if (err)
>  				goto error;
>  			return 0;
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 328985c..29213b5 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -1359,7 +1359,7 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb, u32 features)
>  	fptr = (struct frag_hdr *)(skb_network_header(skb) + unfrag_ip6hlen);
>  	fptr->nexthdr = nexthdr;
>  	fptr->reserved = 0;
> -	ipv6_select_ident(fptr);
> +	ipv6_select_ident(fptr, (struct rt6_info *)skb_dst(skb));
>  
>  	/* Fragment the skb. ipv6 header and the remaining fields of the
>  	 * fragment header are updated in ipv6_gso_segment()
> 


-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[Eric Dumazet]

Le mardi 19 juillet 2011 à 15:56 -0500, Matt Mackall a écrit :
> On Tue, 2011-07-19 at 22:47 +0200, Eric Dumazet wrote:
> > IPv6 fragment identification generation is way beyond what we use for
> > IPv4 : It uses a single generator. Its not scalable and allows DOS
> > attacks.
> > 
> > Now inetpeer is IPv6 aware, we can use it to provide a more secure and
> > scalable frag ident generator (per destination, instead of system wide)
> 
> This code really needs to get moved out of random.c and into net/. Other
> than that, looks fine to me.

Sure. Can we agree to make this cleanup later ?

I added secure_ipv6_id() right after secure_ip_id() because it sounded
the right place ;)

Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[Eric Dumazet]

Le mardi 19 juillet 2011 à 22:47 +0200, Eric Dumazet a écrit :
> IPv6 fragment identification generation is way beyond what we use for
> IPv4 : It uses a single generator. Its not scalable and allows DOS
> attacks.
> 
> Now inetpeer is IPv6 aware, we can use it to provide a more secure and
> scalable frag ident generator (per destination, instead of system wide)
> 
> This patch :
> 1) defines a new secure_ipv6_id() helper
> 2) extends inet_getid() to provide 32bit results
> 3) extends ipv6_select_ident() with a new dest parameter
> 
> Reported-by: Fernando Gont <fernando@gont.com.ar>
> CC: Matt Mackall <mpm@selenic.com>
> CC: Eugene Teo <eugeneteo@kernel.sg>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---


Please hold on, I'll make a different patch series to ease stable teams
job. It appears inetpeer & ipv6 are really not an option for old
kernels.

Common patch for all kernels :
1) Fix the problem without inetpeer help
--- 
Patches for next kernels
2) random split as suggested by Matt Mackal
3) Use inetpeer cache to scale identification generation

Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[Eric Dumazet]

Le mercredi 20 juillet 2011 à 10:25 +0200, Eric Dumazet a écrit :

> Please hold on, I'll make a different patch series to ease stable teams
> job. It appears inetpeer & ipv6 are really not an option for old
> kernels.
> 
> Common patch for all kernels :
> 1) Fix the problem without inetpeer help
> --- 
> Patches for next kernels
> 2) random split as suggested by Matt Mackal
> 3) Use inetpeer cache to scale identification generation
> 

Here is the first patch, applicable on net-2.6 / linux-2.6 and stable
kernels.

Thanks

[PATCH v2] ipv6: make fragment identifications less predictable

Fernando Gont reported current IPv6 fragment identification generation
was not secure, because using a very predictable system-wide generator,
allowing various attacks.

IPv4 uses inetpeer cache to address this problem and to get good
performance. We'll use this mechanism when IPv6 inetpeer is stable
enough in linux-3.1

For the time being, we use jhash on destination address to provide less
predictable identifications. Also remove a spinlock and use cmpxchg() to
get better SMP performance.

Reported-by: Fernando Gont <fernando@gont.com.ar>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Matt Mackall <mpm@selenic.com>
CC: Eugene Teo <eugeneteo@kernel.sg>
---
 include/net/ipv6.h    |   12 -----------
 net/ipv6/ip6_output.c |   43 +++++++++++++++++++++++++++++++++++-----
 net/ipv6/udp.c        |    2 -
 3 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index c033ed0..3b5ac1f 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -463,17 +463,7 @@ static inline int ipv6_addr_diff(const struct in6_addr *a1, const struct in6_add
 	return __ipv6_addr_diff(a1, a2, sizeof(struct in6_addr));
 }
 
-static __inline__ void ipv6_select_ident(struct frag_hdr *fhdr)
-{
-	static u32 ipv6_fragmentation_id = 1;
-	static DEFINE_SPINLOCK(ip6_id_lock);
-
-	spin_lock_bh(&ip6_id_lock);
-	fhdr->identification = htonl(ipv6_fragmentation_id);
-	if (++ipv6_fragmentation_id == 0)
-		ipv6_fragmentation_id = 1;
-	spin_unlock_bh(&ip6_id_lock);
-}
+extern void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt);
 
 /*
  *	Prototypes exported by ipv6
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 9d4b165..8a444c8 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -596,6 +596,38 @@ int ip6_find_1stfragopt(struct sk_buff *skb, u8 **nexthdr)
 	return offset;
 }
 
+static u32 hashidentrnd __read_mostly;
+#define FID_HASH_SZ 16
+static u32 ipv6_fragmentation_id[FID_HASH_SZ];
+
+static int __init initialize_hashidentrnd(void)
+{
+	get_random_bytes(&hashidentrnd, sizeof(hashidentrnd));
+	return 0;
+}
+
+late_initcall_sync(initialize_hashidentrnd);
+
+static u32 __ipv6_select_ident(const struct in6_addr *addr)
+{
+	u32 newid, oldid, hash = jhash2((u32 *)addr, 4, hashidentrnd);
+	u32 *pid = &ipv6_fragmentation_id[hash % FID_HASH_SZ];
+
+	do {
+		oldid = *pid;
+		newid = oldid + 1;
+		if (!(hash + newid))
+			newid++;
+	} while (cmpxchg(pid, oldid, newid) != oldid);
+
+	return hash + newid;
+}
+
+void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt)
+{
+	fhdr->identification = htonl(__ipv6_select_ident(&rt->rt6i_dst.addr));
+}
+
 int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 {
 	struct sk_buff *frag;
@@ -680,7 +712,7 @@ int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 		skb_reset_network_header(skb);
 		memcpy(skb_network_header(skb), tmp_hdr, hlen);
 
-		ipv6_select_ident(fh);
+		ipv6_select_ident(fh, rt);
 		fh->nexthdr = nexthdr;
 		fh->reserved = 0;
 		fh->frag_off = htons(IP6_MF);
@@ -826,7 +858,7 @@ slow_path:
 		fh->nexthdr = nexthdr;
 		fh->reserved = 0;
 		if (!frag_id) {
-			ipv6_select_ident(fh);
+			ipv6_select_ident(fh, rt);
 			frag_id = fh->identification;
 		} else
 			fh->identification = frag_id;
@@ -1072,7 +1104,8 @@ static inline int ip6_ufo_append_data(struct sock *sk,
 			int getfrag(void *from, char *to, int offset, int len,
 			int odd, struct sk_buff *skb),
 			void *from, int length, int hh_len, int fragheaderlen,
-			int transhdrlen, int mtu,unsigned int flags)
+			int transhdrlen, int mtu,unsigned int flags,
+			struct rt6_info *rt)
 
 {
 	struct sk_buff *skb;
@@ -1116,7 +1149,7 @@ static inline int ip6_ufo_append_data(struct sock *sk,
 		skb_shinfo(skb)->gso_size = (mtu - fragheaderlen -
 					     sizeof(struct frag_hdr)) & ~7;
 		skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
-		ipv6_select_ident(&fhdr);
+		ipv6_select_ident(&fhdr, rt);
 		skb_shinfo(skb)->ip6_frag_id = fhdr.identification;
 		__skb_queue_tail(&sk->sk_write_queue, skb);
 
@@ -1282,7 +1315,7 @@ int ip6_append_data(struct sock *sk, int getfrag(void *from, char *to,
 
 			err = ip6_ufo_append_data(sk, getfrag, from, length,
 						  hh_len, fragheaderlen,
-						  transhdrlen, mtu, flags);
+						  transhdrlen, mtu, flags, rt);
 			if (err)
 				goto error;
 			return 0;
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 328985c..29213b5 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1359,7 +1359,7 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb, u32 features)
 	fptr = (struct frag_hdr *)(skb_network_header(skb) + unfrag_ip6hlen);
 	fptr->nexthdr = nexthdr;
 	fptr->reserved = 0;
-	ipv6_select_ident(fptr);
+	ipv6_select_ident(fptr, (struct rt6_info *)skb_dst(skb));
 
 	/* Fragment the skb. ipv6 header and the remaining fields of the
 	 * fragment header are updated in ipv6_gso_segment()

Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[Fernando Gont]

On 07/20/2011 07:27 AM, Eric Dumazet wrote:
> Le mercredi 20 juillet 2011 à 10:25 +0200, Eric Dumazet a écrit :
> 
>> Please hold on, I'll make a different patch series to ease stable teams
>> job. It appears inetpeer & ipv6 are really not an option for old
>> kernels.
>>
>> Common patch for all kernels :
>> 1) Fix the problem without inetpeer help
>> --- 
>> Patches for next kernels
>> 2) random split as suggested by Matt Mackal
>> 3) Use inetpeer cache to scale identification generation
> 
> Here is the first patch, applicable on net-2.6 / linux-2.6 and stable
> kernels.

Does it make sense to go in this direction rather than simply randomize
the IPv6 Fragment Identification?

Keep in mind that IPv6 routers don't perform fragmentation, that that
the IPv6 identification is 32-bits long.

Thanks,
-- 
Fernando Gont
e-mail: fernando@gont.com.ar || fgont@acm.org
PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1

Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[David Miller]

From: Fernando Gont <fernando@gont.com.ar>
Date: Wed, 20 Jul 2011 22:32:18 -0300

> Does it make sense to go in this direction rather than simply randomize
> the IPv6 Fragment Identification?

We could, but that's actually a bit more work.

You have to avoid recycling IDs to the same destination host otherwise
a retransmit could use the same ID and overlap with a previous set of
frags, causing corruption.

This means if you go the "pure random" route, you have to make sure
that the 32-bit series produced by the random number generator is
maximally long.  This is why openbsd uses an ID generator based upon
skip32 etc.

And I cannot say that about our RNG infrastructure.

Also, 32-bits seems like a lot, but on a 40Gb link we can exhaust this
space in ~20 minutes (1554 byte packet over standard ethernet MTU at
40Gbit is ~3454767 ipv6 frag IDs per second).  So while maybe not a
serious issue right now, we seem to go up by a factor of 10 every few
years, therefore at ~400Gb it's down to 2 minutes.

So we have to look at it like a constrained resource, and therefore
doing it on a per-destination basis like ipv4 makes a lot of sense.

I think Eric's work is the way forward and I'll be applying his patches.

Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[Rick Jones]

On 07/21/2011 03:17 PM, David Miller wrote:
> From: Fernando Gont<fernando@gont.com.ar>
> Date: Wed, 20 Jul 2011 22:32:18 -0300
>
>> Does it make sense to go in this direction rather than simply randomize
>> the IPv6 Fragment Identification?
>
> We could, but that's actually a bit more work.
>
> You have to avoid recycling IDs to the same destination host otherwise
> a retransmit could use the same ID and overlap with a previous set of
> frags, causing corruption.

I think you mean ID reuse rather than packet retransmit no?

It is the same "frankengram" issue present in IPv4 with its now puny 16 
bit id field. Doesn't that pretty much rely on layer4 or higher 
checksums to avoid corruption?

> This means if you go the "pure random" route, you have to make sure
> that the 32-bit series produced by the random number generator is
> maximally long.  This is why openbsd uses an ID generator based upon
> skip32 etc.
>
> And I cannot say that about our RNG infrastructure.
>
> Also, 32-bits seems like a lot, but on a 40Gb link we can exhaust this
> space in ~20 minutes (1554 byte packet over standard ethernet MTU at
> 40Gbit is ~3454767 ipv6 frag IDs per second).  So while maybe not a
> serious issue right now, we seem to go up by a factor of 10 every few
> years, therefore at ~400Gb it's down to 2 minutes.

With mode-rr and bonding (and enough CPU) you could probably get it down 
to 2 minutes without having to wait a few years.

rick jones

> So we have to look at it like a constrained resource, and therefore
> doing it on a per-destination basis like ipv4 makes a lot of sense.
>
> I think Eric's work is the way forward and I'll be applying his patches.
> --
> 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

Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[David Miller]

From: Rick Jones <rick.jones2@hp.com>
Date: Thu, 21 Jul 2011 15:46:21 -0700

> On 07/21/2011 03:17 PM, David Miller wrote:
>> From: Fernando Gont<fernando@gont.com.ar>
>> Date: Wed, 20 Jul 2011 22:32:18 -0300
>>
>>> Does it make sense to go in this direction rather than simply
>>> randomize
>>> the IPv6 Fragment Identification?
>>
>> We could, but that's actually a bit more work.
>>
>> You have to avoid recycling IDs to the same destination host otherwise
>> a retransmit could use the same ID and overlap with a previous set of
>> frags, causing corruption.
> 
> I think you mean ID reuse rather than packet retransmit no?
> 
> It is the same "frankengram" issue present in IPv4 with its now puny
> 16 bit id field. Doesn't that pretty much rely on layer4 or higher
> checksums to avoid corruption?

We've had documented cases where checksums match after recycling the
fragment ID space in ipv4, that's why we have all of the special
code in the ipv4 fragmentation handling to work around that problem.

Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[Fernando Gont]

On 07/21/2011 07:17 PM, David Miller wrote:
>> Does it make sense to go in this direction rather than simply randomize
>> the IPv6 Fragment Identification?
> 
> We could, but that's actually a bit more work.
> 
> You have to avoid recycling IDs to the same destination host otherwise
> a retransmit could use the same ID and overlap with a previous set of
> frags, causing corruption.
> 
> This means if you go the "pure random" route, you have to make sure
> that the 32-bit series produced by the random number generator is
> maximally long.  This is why openbsd uses an ID generator based upon
> skip32 etc.

That scenario assumes packet reordering and/or packet loss. In that
case, and at those packet rates, I'd argue that you shoudn't be relying
on fragmentation, anyway (e.g., use PMTUD).


> So we have to look at it like a constrained resource, and therefore
> doing it on a per-destination basis like ipv4 makes a lot of sense.
> 
> I think Eric's work is the way forward and I'll be applying his patches.

While I haven't had a chance to look at the patch yet, I was wondering
whether it defines both a separe "offset" and "counter" for each "flow".

(assuming that Identification values are selected as a result of ID =
offset + counter, where counter is incremented for each packet that is
sent, and offset is some value that depends on (src IP, dst ip)).

If you have separate "offset" for each flow, you solve the problem of
"predictable Identification values". However, if there's a single global
"counter", you're still subject of being exploited for an "idle scan").

Thanks,
-- 
Fernando Gont
e-mail: fernando@gont.com.ar || fgont@acm.org
PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1

Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[David Miller]

From: Fernando Gont <fernando@gont.com.ar>
Date: Thu, 21 Jul 2011 20:37:48 -0300

> While I haven't had a chance to look at the patch yet, I was wondering
> whether it defines both a separe "offset" and "counter" for each "flow".

Why wonder, just take a look.

If you aren't willing to even bother looking at the solution we came
up with, why should we feel compelled to spend any time at all looking
at your "anaylsis" and "concerns"?

Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[Rick Jones]

On 07/21/2011 04:37 PM, Fernando Gont wrote:
> On 07/21/2011 07:17 PM, David Miller wrote:
>>> Does it make sense to go in this direction rather than simply randomize
>>> the IPv6 Fragment Identification?
>>
>> We could, but that's actually a bit more work.
>>
>> You have to avoid recycling IDs to the same destination host otherwise
>> a retransmit could use the same ID and overlap with a previous set of
>> frags, causing corruption.
>>
>> This means if you go the "pure random" route, you have to make sure
>> that the 32-bit series produced by the random number generator is
>> maximally long.  This is why openbsd uses an ID generator based upon
>> skip32 etc.
>
> That scenario assumes packet reordering and/or packet loss.

Isn't that a given?  I mean if there were no packet reordering or packet 
loss then the size of the ID space wouldn't matter right? The fragments 
would be sent, in order and without loss and all would be happiness and 
joy.  It is only because there is packet reordering and/or packet loss 
that we care about the size of the ID space and the time to reuse of a 
given ID.

And indeed, fragmentation is considered bad, and was considered bad 
enough that the "revenge of the router guys" that is IPv6 punted it to 
the end systems, and yes, one should use PMTUD. Which is all well and 
good when 999 times out of 1 traffic is flowing over a transport that 
does its own segmentation and reassembly.  And when IPv6 got spec'ed it 
looked to all the world that UDP was on the way out - NFS was migrating 
over to TCP, and DNS was "never" more than 512 byte messages. No problem 
right?  But since then we've gotten things like EDNS which will be 
sending DNS messages in UDP datagrams that will have to be fragmented, 
PMTUD notwithstanding.

rick jones
almost certainly fumbled a TLA in there somewhere :)

Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[Fernando Gont]

On 07/21/2011 09:34 PM, Rick Jones wrote:
>> That scenario assumes packet reordering and/or packet loss.
> 
> Isn't that a given?  

I mean that if these "collisions" of Identification numbers are of
concern, then, then, at such bandwidth rates, fragmentation itself would
be a concern.

Chances of collisions are proportional to reordering and losses. That
means that at such bandwidths, you'd need to be able to queue a huge
number of packets (which you might not be able to queue, becacuse of
lack of resources), etc.



> And indeed, fragmentation is considered bad, and was considered bad
> enough that the "revenge of the router guys" that is IPv6 punted it to
> the end systems, and yes, one should use PMTUD. Which is all well and
> good when 999 times out of 1 traffic is flowing over a transport that
> does its own segmentation and reassembly.  

And provided that there's no ICMPv6 filtering out there (which there is)
-- at which point you need to implement some for of blackhole detection
a la PLMPTUD.


> And when IPv6 got spec'ed it
> looked to all the world that UDP was on the way out - NFS was migrating
> over to TCP, and DNS was "never" more than 512 byte messages. No problem
> right?  But since then we've gotten things like EDNS which will be
> sending DNS messages in UDP datagrams that will have to be fragmented,
> PMTUD notwithstanding.

Hopefully you won't have the aforementioned 40GB traffic rate between
two DNS servers ;-)

Thanks,
-- 
Fernando Gont
e-mail: fernando@gont.com.ar || fgont@acm.org
PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1

Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 19 Jul 2011 22:47:03 +0200

> IPv6 fragment identification generation is way beyond what we use for
> IPv4 : It uses a single generator. Its not scalable and allows DOS
> attacks.
> 
> Now inetpeer is IPv6 aware, we can use it to provide a more secure and
> scalable frag ident generator (per destination, instead of system wide)
> 
> This patch :
> 1) defines a new secure_ipv6_id() helper
> 2) extends inet_getid() to provide 32bit results
> 3) extends ipv6_select_ident() with a new dest parameter
> 
> Reported-by: Fernando Gont <fernando@gont.com.ar>
> CC: Matt Mackall <mpm@selenic.com>
> CC: Eugene Teo <eugeneteo@kernel.sg>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, and I'll queue up your backport-friendly version for -stable.