handle overlapping IPv6 fragments

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

handle overlapping IPv6 fragments

Alexander Bluhm
Hi,

Implement RFC 5722 and drop all IPv6 fragments that belong to a
packet with overlapping fragments.

ok?

bluhm


Index: netinet6/frag6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/frag6.c,v
retrieving revision 1.39
diff -u -p -r1.39 frag6.c
--- netinet6/frag6.c 10 Jan 2012 12:50:32 -0000 1.39
+++ netinet6/frag6.c 10 Jan 2012 16:15:46 -0000
@@ -284,8 +284,15 @@ frag6_input(struct mbuf **mp, int *offp,
  q6->ip6q_src = ip6->ip6_src;
  q6->ip6q_dst = ip6->ip6_dst;
  q6->ip6q_unfrglen = -1; /* The 1st fragment has not arrived. */
-
- q6->ip6q_nfrag = 0;
+ q6->ip6q_nfrag  = 0;
+ } else if (LIST_EMPTY(&q6->ip6q_asfrag)) {
+ /*
+ * Overlapping fragments have been detected.  Do not
+ * reassemble packet but also drop future fragments.
+ * This will be done for this ident/src/dst combination
+ * until fragment queue timeout.
+ */
+ goto dropfrag;
  }
 
  /*
@@ -402,11 +409,10 @@ frag6_input(struct mbuf **mp, int *offp,
  break;
 
  /*
- * If the incoming fragment overlaps some existing fragments in
- * the reassembly queue, drop it, since it is dangerous to override
- * existing fragments from a security point of view.
- * We don't know which fragment is the bad guy - here we trust
- * fragment that came in earlier, with no real reason.
+ * RFC5722:  When reassembling an IPv6 datagram, if one or more its
+ * constituent fragments is determined to be an overlapping fragment,
+ * the entire datagram (and any constituent fragments, including those
+ * not yet received) MUST be silently discarded.
  */
  if (paf6 != LIST_END(&q6->ip6q_asfrag)) {
  i = (paf6->ip6af_off + paf6->ip6af_frglen) - ip6af->ip6af_off;
@@ -417,7 +423,7 @@ frag6_input(struct mbuf **mp, int *offp,
     i, ip6_sprintf(&q6->ip6q_src));
 #endif
  free(ip6af, M_FTABLE);
- goto dropfrag;
+ goto flushfrags;
  }
  }
  if (af6 != LIST_END(&q6->ip6q_asfrag)) {
@@ -429,7 +435,7 @@ frag6_input(struct mbuf **mp, int *offp,
     i, ip6_sprintf(&q6->ip6q_src));
 #endif
  free(ip6af, M_FTABLE);
- goto dropfrag;
+ goto flushfrags;
  }
  }
 
@@ -534,6 +540,17 @@ frag6_input(struct mbuf **mp, int *offp,
 
  IP6Q_UNLOCK();
  return nxt;
+
+ flushfrags:
+ while ((af6 = LIST_FIRST(&q6->ip6q_asfrag)) !=
+    LIST_END(&q6->ip6q_asfrag)) {
+ LIST_REMOVE(af6, ip6af_list);
+ m_freem(IP6_REASS_MBUF(af6));
+ free(af6, M_FTABLE);
+ }
+ ip6stat.ip6s_fragdropped += q6->ip6q_nfrag;
+ frag6_nfrags -= q6->ip6q_nfrag;
+ q6->ip6q_nfrag = 0;
 
  dropfrag:
  in6_ifstat_inc(dstifp, ifs6_reass_fail);

Reply | Threaded
Open this post in threaded view
|

Re: handle overlapping IPv6 fragments

Fernando Gont-2
On 01/10/2012 01:20 PM, Alexander Bluhm wrote:
> Implement RFC 5722 and drop all IPv6 fragments that belong to a
> packet with overlapping fragments.

FWIW, you may be interested in this one, too:
http://tools.ietf.org/id/draft-gont-6man-ipv6-atomic-fragments-00.txt

Thanks,
--
Fernando Gont
e-mail: [hidden email] || [hidden email]
PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1

Reply | Threaded
Open this post in threaded view
|

Re: handle overlapping IPv6 fragments

Alexander Bluhm
On Tue, Jan 10, 2012 at 07:51:03PM -0300, Fernando Gont wrote:
> On 01/10/2012 01:20 PM, Alexander Bluhm wrote:
> > Implement RFC 5722 and drop all IPv6 fragments that belong to a
> > packet with overlapping fragments.
>
> FWIW, you may be interested in this one, too:
> http://tools.ietf.org/id/draft-gont-6man-ipv6-atomic-fragments-00.txt

I already was aware of it.  It makes sense to me.

Do we want this in our stack although it is not an RFC yet?
Or perhaps only in pf for extra security?

bluhm


Index: netinet6/frag6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/frag6.c,v
retrieving revision 1.40
diff -u -p -r1.40 frag6.c
--- netinet6/frag6.c 10 Jan 2012 17:09:02 -0000 1.40
+++ netinet6/frag6.c 11 Jan 2012 02:43:55 -0000
@@ -235,6 +235,20 @@ frag6_input(struct mbuf **mp, int *offp,
  /* offset now points to data portion */
  offset += sizeof(struct ip6_frag);
 
+ /*
+ * draft-gont-6man-ipv6-atomic-fragments-00:  A host that receives an
+ * IPv6 packet which includes a Fragment Header with the "Fragment
+ * Offset" equal to 0 and the "M" bit equal to 0 MUST process such
+ * packet in isolation from any other packets/fragments.
+ */
+ fragoff = ntohs(ip6f->ip6f_offlg & IP6F_OFF_MASK);
+ if (fragoff == 0 && !(ip6f->ip6f_offlg & IP6F_MORE_FRAG)) {
+ ip6stat.ip6s_reassembled++;
+ in6_ifstat_inc(dstifp, ifs6_reass_ok);
+ *offp = offset;
+ return ip6f->ip6f_nxt;
+ }
+
  IP6Q_LOCK();
 
  /*
@@ -299,7 +313,6 @@ frag6_input(struct mbuf **mp, int *offp,
  * If it's the 1st fragment, record the length of the
  * unfragmentable part and the next header of the fragment header.
  */
- fragoff = ntohs(ip6f->ip6f_offlg & IP6F_OFF_MASK);
  if (fragoff == 0) {
  q6->ip6q_unfrglen = offset - sizeof(struct ip6_hdr) -
     sizeof(struct ip6_frag);

Reply | Threaded
Open this post in threaded view
|

Re: handle overlapping IPv6 fragments

Fernando Gont-2
On 01/11/2012 12:16 AM, Alexander Bluhm wrote:

> On Tue, Jan 10, 2012 at 07:51:03PM -0300, Fernando Gont wrote:
>> On 01/10/2012 01:20 PM, Alexander Bluhm wrote:
>>> Implement RFC 5722 and drop all IPv6 fragments that belong to a
>>> packet with overlapping fragments.
>>
>> FWIW, you may be interested in this one, too:
>> http://tools.ietf.org/id/draft-gont-6man-ipv6-atomic-fragments-00.txt
>
> I already was aware of it.  It makes sense to me.
>
> Do we want this in our stack although it is not an RFC yet?
> Or perhaps only in pf for extra security?

I should note that an RFC can take at least a year to publish (if ever).

So far, to the extent that the aforementioned I-D has been discussed on
the IETF 6man mailing-list, I haven't seen anybody opposing to it and,
on the other hand, quite a few people have expressed their support. So
I'd argue that you should apply this patch, and be done with it.

P.S.: Clearly, I'm biased, since I'm the author of the I-D, but...

Thanks,
--
Fernando Gont
e-mail: [hidden email] || [hidden email]
PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1

Reply | Threaded
Open this post in threaded view
|

Re: handle overlapping IPv6 fragments

Simon Perreault-3
On 01/12/2012 03:39 AM, Fernando Gont wrote:
>> Do we want this in our stack although it is not an RFC yet?
>> Or perhaps only in pf for extra security?
>
> I should note that an RFC can take at least a year to publish (if ever).

We should not wait for an RFC. We should wait for a consensus to emerge.
This can be quick, and I think we're almost there. So I think this patch
can safely be committed now.

Simon
--
DTN made easy, lean, and smart --> http://postellation.viagenie.ca
NAT64/DNS64 open-source        --> http://ecdysis.viagenie.ca
STUN/TURN server               --> http://numb.viagenie.ca