1 Topic by jonas (edited by jonas 2013-05-12 23:42:11)

  • Registered: 2013-05-12
  • Posts: 4

Topic: DTLS crash after timeout (patch)

We have had problems with wolfSSL embedded ssl 2.6 crashing after negotiations fail. It turns out that it tried to free a buffer that was never malloced, and I traced the problem to DtlsPoolSend. It should call CheckAvalaibleSize() [sic], shouldn't it?

The below patch (on wolfssl-2.6.0) appears to solve the problem. Looking at the code, building without LARGE_STATIC_BUFFERS (which is what I do) makes things worse.

Index: cyassl/src/internal.c
===================================================================
--- cyassl.orig/src/internal.c    2013-05-08 15:22:15.704663345 +0200
+++ cyassl/src/internal.c    2013-05-08 15:24:14.716658339 +0200
@@ -1791,6 +1791,7 @@
 
 int DtlsPoolSend(CYASSL* ssl)
 {
+    int ret;
     DtlsPool *pool = ssl->dtls_pool;
 
     if (pool != NULL && pool->used > 0) {
@@ -1807,6 +1808,9 @@
             c16toa(ssl->keys.dtls_epoch, dtls->epoch);
             c32to48(ssl->keys.dtls_sequence_number++, dtls->sequence_number);
 
+            if ((ret = CheckAvalaibleSize(ssl, buf->length)) != 0)
+                return ret;
+
             XMEMCPY(ssl->buffers.outputBuffer.buffer, buf->buffer, buf->length);
             ssl->buffers.outputBuffer.idx = 0;
             ssl->buffers.outputBuffer.length = buf->length;

Share

  • john
  • Moderator
  • Offline
  • From: Seattle, WA
  • Registered: 2011-11-14
  • Posts: 140

Re: DTLS crash after timeout (patch)

Thanks for the catch! I have pushed this change to our GitHub repository as commit ac716c9.

john's Website

Share

  • Registered: 2013-05-12
  • Posts: 4

Re: DTLS crash after timeout (patch)

Thanks John!

While digging through the DTLS code (2.6.0) I've found two other things that got me scratching my head:

1. SendFinished stores the Finished message twice in the pool: first something is saved by BuildMessage and then SendFinished calls DtlsPoolSave directly. It looks like BuildMessage shouldn't do that, and the resulting message on the wire is broken.

2. The epoch is incremented in an inconsistent way. When a ChangeCipherSpec is transmitted in the regular manner it is sent with the old epoch, and the next message uses the new epoch number. When DtlsPoolSend makes a retransmission of a ChangeCipherSpec the epoch is incremented first and the the CCS message uses the new epoch.

Share

  • john
  • Moderator
  • Offline
  • From: Seattle, WA
  • Registered: 2011-11-14
  • Posts: 140

Re: DTLS crash after timeout (patch)

Thanks again. I have pushed this change to our GitHub repository as commit 05f11c4.

john's Website

Share

5 Reply by jonas (edited by jonas 2013-05-20 05:43:35)

  • Registered: 2013-05-12
  • Posts: 4

Re: DTLS crash after timeout (patch)

Hi,

Your solution to the second (epoch) issue isn't going to work. The record layer sequence number should be incremented also on retransmitted packets, says the RFC. What's less clear is what to do with epoch/sequence number on the Finished message; if the receiver never got the CCS+Finished flight, did the epoch change ever happen? What OpenSSL does here is to send Finished with the new epoch, but wait until the other end has answered before actually changing its state. The result is that the Finished message of the initial handshake is always sent as epoch 1, seqno 0, no matter how many retransmissions are made. I don't know how mature that DTLS implementation is, but to me it feels like a sensible way to go about epochs and sequence numbers.

I have implemented this and tested it against the OpenSSL example server, dropping different packets during the handshake: https://github.com/JonasNorling/cyassl/ … c903aadd26. I have posted a GitHub pull request.

Share

  • john
  • Moderator
  • Offline
  • From: Seattle, WA
  • Registered: 2011-11-14
  • Posts: 140

Re: DTLS crash after timeout (patch)

Thank you for the patch. I'll incorporate it when I get a chance to look it over.

john's Website

Share