On Apr 1, 2011, at 12:25 PM, Ben Laurie wrote: > On 01/04/2011 09:02, Robin Seggelmann via RT wrote: >> Hi, >> >> On Apr 1, 2011, at 9:28 AM, via RT wrote: >> >>> I’ve tested DTLS implementation and know that several fixes has >>> been applied for issues related to fragment. >> >> Thanks for testing! There is a known issue with the bitmask, the >> patch #2457 addresses that, but has not been applied to the official >> source yet. > > I'd be happier about applying these patches if there were tests that > showed the brokenness without them ... are there?
The problem can be easily calculated. In d1_both.c, line 143: if (bitmask[(((msg_len) - 1) >> 3)] != bitmask_end_values[((msg_len) & 7)]) is_complete = 0; This checks if the last byte of the bitmask has not yet every necessary bit set, and thus not all bytes of the message have been received and more fragments are necessary. If the total length of the message is a multiple of 8, then msg_len & 7 will result in 0, therefore bitmask_end_values[0] = 0x00 will be compared with the last byte of the bitmask. With a length being a multiple of 8, however, the last byte will be used completely and therefore has to be compared with 0xff to check if everything has been received. Two examples: Total message length is 12, therefore the bitmask is 2 bytes. The first byte is used entirely, but only 4 bits of the second. The if clause mentioned above will look like that: bitmask[(((12) - 1) >> 3)] != bitmask_end_values[((12) & 7)] bitmask[1] != bitmask_end_values[4] bitmask_end_values[4] = 0x0f That is, the first 4 bits of the byte have to be set, which is exactly as expected. Total message length is 16, therefore the bitmask is 2 bytes again. Both bytes will be used entirely. The if clause mentioned above will look like that: bitmask[(((16) - 1) >> 3)] != bitmask_end_values[((16) & 7)] bitmask[1] != bitmask_end_values[0] bitmask_end_values[4] = 0x00 That is, the second byte has to be all zero to pass the check if the message is complete, what is obviously not what we want. So in this case the check only declares a message as complete if the last 8 bytes are missing. Therefore, the bitmask_end_values[] array has to be changed as done in the patch #2457. For patch #2458 there is a bug report on the users mailing list from Neeraj Rajgure, the subject was "Server not accepting connections after too many client connections". He confirmed that the patch solves the issues he found. For patch #2462 there is a bug report and its discussion on the developer mailing list from Robert Story, the subject was "DTLS OpenSSL internal error, assertion failed: i == DTLS1_HM_HEADER_LENGTH". He promised testing the patch but has not responded until now, so I guess it's working. I was able to reproduce these bugs and the patches have been tested in our lab and on our interop server, which is frequently tested with a commercial implementation of DTLS. A summary of our patches is available on http://sctp.fh-muenster.de/dtls-patches.html, including their descriptions. -Robin PS: Is there any chance for getting commit rights, so I can fix DTLS issues much more easily? :) >>> But, it still has a problem to reassemble fragments. Please, check >>> the attached patch. >> >> Patch #2457 corrects the first value in bitmask_end_values[] to 0xff. >> Your patch is basically doing the same, but you also shift the entire >> array, which requires adjusting the index when accessing it by >> subtracting one. Please have a look at http://sctp.fh-muenster.de for >> the latest DTLS patches. It would be great if you could confirm that >> #2457 fixes the problems you encountered. >> >> Best regards Robin >> >> >> >> >> >> >> ______________________________________________________________________ >> >> > OpenSSL Project http://www.openssl.org >> Development Mailing List >> [email protected] Automated List Manager >> [email protected] >> >> > > > -- > http://www.apache-ssl.org/ben.html http://www.links.org/ > > "There is no limit to what a man can do or how far he can go if he > doesn't mind who gets the credit." - Robert Woodruff > ______________________________________________________________________ > OpenSSL Project http://www.openssl.org > Development Mailing List [email protected] > Automated List Manager [email protected] ______________________________________________________________________ OpenSSL Project http://www.openssl.org Development Mailing List [email protected] Automated List Manager [email protected]
