Quantcast

DTLS: doubts about fragment reassembler

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

DTLS: doubts about fragment reassembler

Andreas Schultz
Hi Ingela,

I tried to understand the fragment reassembler in dtls_handshake.erl
and it seems that it is assuming that retransmissions will always use
the same record size.

RFC 6347, Sect. 4.3.2 states:

> When a DTLS implementation receives a handshake message fragment, it
> MUST buffer it until it has the entire handshake message.  DTLS
> implementations MUST be able to handle overlapping fragment ranges.
> This allows senders to retransmit handshake messages with smaller
> fragment sizes if the PMTU estimate changes.

Last time I looked at OpenSSL's DTLS sending side, it did exactly that.
It refragmented the handshake on retransmit to the minimum IP MTU.

From my understanding of the fragment merging logic, it will not merge
overlapping fragments (it might even become confused).

Am I missing something there?

Regards
Andreas
_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: DTLS: doubts about fragment reassembler

Andreas Schultz


----- On Mar 9, 2017, at 10:00 PM, Ingela Andin <[hidden email]> wrote:
Hi Andreas!

On Thu, Mar 9, 2017 at 7:54 PM, Andreas Schultz <[hidden email]> wrote:
Hi Ingela,

I tried to understand the fragment reassembler in dtls_handshake.erl
and it seems that it is assuming that retransmissions will always use
the same record size.


No it is not assuming that. It just does not care what the size is, it suppose
to just fill in the gaps (in the merge) until it has reasemmbled a whole. 
Of course I have not  written any specific test for it yet so I am not saying it
may not have bugs. 

I'm staring at dtls_handshake:merge_fragments/2 and I can't convince
myself that it will merge overlapping fragments.

Let's assume we have <<0,1,2>> and <4,5,6>>, the new fragment is <<2,3,4>>.
This would mean the following fragment records:

1: #handshake_fragment{fragment_offset = 0, fragment_length = 3, fragment = <<0,1,2>>}
2: #handshake_fragment{fragment_offset = 2, fragment_length = 3, fragment = <<2,3,4>>}
3: #handshake_fragment{fragment_offset = 4, fragment_length = 3, fragment = <<4,5,6>>}

IMHO merge_fragments will not merge fragment #1 and #2 (none of the conditions match),
the same goes for fragment #2 and #3.

The expected output would be a fragment:

 #handshake_fragment{fragment_offset = 0, fragment_length = 6, fragment = <<0,1,2,3,4,5,6>>}


I think the following (untested) change is needed:

diff --git a/lib/ssl/src/dtls_handshake.erl b/lib/ssl/src/dtls_handshake.erl
index fd1f969..245dc6e 100644
--- a/lib/ssl/src/dtls_handshake.erl
+++ b/lib/ssl/src/dtls_handshake.erl
@@ -464,10 +464,28 @@ merge_fragments(#handshake_fragment{
                #handshake_fragment{
                   fragment_offset = CurrentOffSet,
                   fragment_length = CurrentLen,
-                  fragment = CurrentData}) when PreviousOffSet + PreviousLen == CurrentOffSet->
-           Previous#handshake_fragment{
-             fragment_length =  PreviousLen + CurrentLen,
-             fragment = <<PreviousData/binary, CurrentData/binary>>};
+                  fragment = CurrentData})
+  when PreviousOffSet + PreviousLen >= CurrentOffSet andalso
+       PreviousOffSet + PreviousLen < CurrentOffSet + CurrentLength ->
+    CurrentStart = PreviousOffSet + PreviousLen - CurrentOffSet,
+    <<_:CurrentStart/bytes, Data/binary>> = CurrentData,
+    Previous#handshake_fragment{
+      fragment_length =  PreviousLen + CurrentLen - CurrentStart,
+      fragment = <<PreviousData/binary, Data/binary>>};
+%% already fully contained fragment
+merge_fragments(#handshake_fragment{
+                  fragment_offset = PreviousOffSet, 
+                  fragment_length = PreviousLen,
+                  fragment = PreviousData
+                 } = Previous, 
+               #handshake_fragment{
+                  fragment_offset = CurrentOffSet,
+                  fragment_length = CurrentLen,
+                  fragment = CurrentData})
+  when PreviousOffSet + PreviousLen >= CurrentOffSet andalso
+       PreviousOffSet + PreviousLen >= CurrentOffSet + CurrentLength ->
+    Previous;
+

Andreas

RFC 6347, Sect. 4.3.2 states:

> When a DTLS implementation receives a handshake message fragment, it
> MUST buffer it until it has the entire handshake message.  DTLS
> implementations MUST be able to handle overlapping fragment ranges.
> This allows senders to retransmit handshake messages with smaller
> fragment sizes if the PMTU estimate changes.

Last time I looked at OpenSSL's DTLS sending side, it did exactly that.
It refragmented the handshake on retransmit to the minimum IP MTU.

From my understanding of the fragment merging logic, it will not merge
overlapping fragments (it might even become confused).

Am I missing something there?


It is suppose to merge overlapping fragments. I am currently trying to get
all the basic tests going.  I acctually merged some new code this week
with a few fixes and a lot more tests activated, however I doubt that the nighlty
tests drops a lot of packages....

There are still a lot to do but now I think we are getting there. And I had a lot of
use of your code even the parts I did not use. I hope to get some quick check 
tests going too in the long run.

Regards Ingela 



_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Loading...