[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