[erlang-questions] DTLS: doubts about fragment reassembler

Andreas Schultz aschultz@REDACTED
Thu Mar 9 22:39:38 CET 2017


----- On Mar 9, 2017, at 10:00 PM, Ingela Andin <ingela@REDACTED> wrote: 

> Hi Andreas!

> On Thu, Mar 9, 2017 at 7:54 PM, Andreas Schultz < [ mailto:aschultz@REDACTED |
> aschultz@REDACTED ] > 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 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-questions/attachments/20170309/0e123b86/attachment.htm>


More information about the erlang-questions mailing list