| Message ID | 1311108423.3113.24.camel@edumazet-laptop |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
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() >
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 ;) -- 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
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 -- 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
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. -- 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
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()
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(-) -- 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