{"action":"created","issue":{"url":"https://api.github.com/repos/ARMmbed/mbedtls/issues/345","labels_url":"https://api.github.com/repos/ARMmbed/mbedtls/issues/345/labels{/name}","comments_url":"https://api.github.com/repos/ARMmbed/mbedtls/issues/345/comments","events_url":"https://api.github.com/repos/ARMmbed/mbedtls/issues/345/events","html_url":"https://github.com/ARMmbed/mbedtls/issues/345","id":114693156,"number":345,"title":"DTLS datagram drop on single record header epoch failure","user":{"login":"jeannotlapin","id":10604519,"avatar_url":"https://avatars.githubusercontent.com/u/10604519?v=3","gravatar_id":"","url":"https://api.github.com/users/jeannotlapin","html_url":"https://github.com/jeannotlapin","followers_url":"https://api.github.com/users/jeannotlapin/followers","following_url":"https://api.github.com/users/jeannotlapin/following{/other_user}","gists_url":"https://api.github.com/users/jeannotlapin/gists{/gist_id}","starred_url":"https://api.github.com/users/jeannotlapin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jeannotlapin/subscriptions","organizations_url":"https://api.github.com/users/jeannotlapin/orgs","repos_url":"https://api.github.com/users/jeannotlapin/repos","events_url":"https://api.github.com/users/jeannotlapin/events{/privacy}","received_events_url":"https://api.github.com/users/jeannotlapin/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/ARMmbed/mbedtls/labels/bug","name":"bug","color":"fc2929"},{"url":"https://api.github.com/repos/ARMmbed/mbedtls/labels/closed_in_jira","name":"closed_in_jira","color":"ededed"},{"url":"https://api.github.com/repos/ARMmbed/mbedtls/labels/mirrored","name":"mirrored","color":"ededed"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2015-11-02T21:57:33Z","updated_at":"2015-12-16T12:30:34Z","closed_at":null,"body":"When the client is resending packets after a server reply is lost, it resends the last 5 records.\r\n\r\n- If client is polarssl, it will issue 5 datagrams with a record in each one of them. The first 4 will be drop by server for invalid epoch(they have 0 and server is now expecting 1) in record header, and the last one, the encrypted handshake message has epoch=1 so when routed to ssl_read, it will make the server resending its last two packets.\r\n\r\n- When the peer client is Chrome, it resends the 5 records in one datagram, but the first one having a bad header(invalid epoch), the whole datagram is dropped.\r\n\r\n\r\n\r\nSee discussion about the bug there: https://tls.mbed.org/discussions/bug-report-issues/dtls-packet-loss-management\r\n\r\nPlease found a patch applying to polarssl1.4 patched to get SRTP-DTLS key generation. One important thing is to mode the epoch checking in ssl_parse_record_header() before the check for changeCipherSpec check otherwise that one may trigger and in result drop the whole datagram.\r\n\r\nI can't attach the file,  so here is the patch pasted as text:\r\n\r\n```diff\r\nFrom 919006f5f33febe72205078a125fae811b12a3d6 Mon Sep 17 00:00:00 2001\r\nFrom: Johan Pascal <johan.pascal@belledonne-communications.com>\r\nDate: Fri, 30 Oct 2015 16:02:06 +0100\r\nSubject: [PATCH 1/1] Workaround a bug dropping whole DTLS datagram when one\r\n record is out of epoch\r\n\r\n---\r\n include/polarssl/ssl.h |  1 +\r\n library/ssl_tls.c      | 57 ++++++++++++++++++++++++++------------------------\r\n 2 files changed, 31 insertions(+), 27 deletions(-)\r\n\r\ndiff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h\r\nindex e0c2df5..1f014b8 100644\r\n--- a/include/polarssl/ssl.h\r\n+++ b/include/polarssl/ssl.h\r\n@@ -113,6 +113,7 @@\r\n #define POLARSSL_ERR_SSL_BAD_INPUT_DATA                    -0x7100  /**< Bad input parameters to function. */\r\n #define POLARSSL_ERR_SSL_INVALID_MAC                       -0x7180  /**< Verification of the message MAC failed. */\r\n #define POLARSSL_ERR_SSL_INVALID_RECORD                    -0x7200  /**< An invalid SSL record was received. */\r\n+#define POLARSSL_ERR_SSL_INVALID_RECORD_EPOCH              -0x7240  /**< An invalid SSL record was received whith wrong epoch. */\r\n #define POLARSSL_ERR_SSL_CONN_EOF                          -0x7280  /**< The connection indicated an EOF. */\r\n #define POLARSSL_ERR_SSL_UNKNOWN_CIPHER                    -0x7300  /**< An unknown cipher was received. */\r\n #define POLARSSL_ERR_SSL_NO_CIPHER_CHOSEN                  -0x7380  /**< The server has no ciphersuites in common with the client. */\r\ndiff --git a/library/ssl_tls.c b/library/ssl_tls.c\r\nindex 60c5b4c..624003c 100644\r\n--- a/library/ssl_tls.c\r\n+++ b/library/ssl_tls.c\r\n@@ -3020,30 +3020,6 @@ static int ssl_parse_record_header( ssl_context *ssl )\r\n         return( POLARSSL_ERR_SSL_INVALID_RECORD );\r\n     }\r\n \r\n-#if defined(POLARSSL_SSL_PROTO_DTLS)\r\n-    if( ssl->transport == SSL_TRANSPORT_DATAGRAM )\r\n-    {\r\n-        /* Drop unexpected ChangeCipherSpec messages */\r\n-        if( ssl->in_msgtype == SSL_MSG_CHANGE_CIPHER_SPEC &&\r\n-            ssl->state != SSL_CLIENT_CHANGE_CIPHER_SPEC &&\r\n-            ssl->state != SSL_SERVER_CHANGE_CIPHER_SPEC )\r\n-        {\r\n-            SSL_DEBUG_MSG( 1, ( \"dropping unexpected ChangeCipherSpec\" ) );\r\n-            return( POLARSSL_ERR_SSL_INVALID_RECORD );\r\n-        }\r\n-\r\n-        /* Drop unexpected ApplicationData records,\r\n-         * except at the beginning of renegotiations */\r\n-        if( ssl->in_msgtype == SSL_MSG_APPLICATION_DATA &&\r\n-            ssl->state != SSL_HANDSHAKE_OVER &&\r\n-            ! ( ssl->renegotiation == SSL_RENEGOTIATION &&\r\n-                ssl->state == SSL_SERVER_HELLO ) )\r\n-        {\r\n-            SSL_DEBUG_MSG( 1, ( \"dropping unexpected ApplicationData\" ) );\r\n-            return( POLARSSL_ERR_SSL_INVALID_RECORD );\r\n-        }\r\n-    }\r\n-#endif\r\n \r\n     /* Check version */\r\n     if( major_ver != ssl->major_ver )\r\n@@ -3069,7 +3045,7 @@ static int ssl_parse_record_header( ssl_context *ssl )\r\n             SSL_DEBUG_MSG( 1, ( \"record from another epoch: \"\r\n                                 \"expected %d, received %d\",\r\n                                  ssl->in_epoch, rec_epoch ) );\r\n-            return( POLARSSL_ERR_SSL_INVALID_RECORD );\r\n+            return( POLARSSL_ERR_SSL_INVALID_RECORD_EPOCH );\r\n         }\r\n \r\n #if defined(POLARSSL_SSL_DTLS_ANTI_REPLAY)\r\n@@ -3080,6 +3056,29 @@ static int ssl_parse_record_header( ssl_context *ssl )\r\n         }\r\n #endif\r\n     }\r\n+\r\n+    if( ssl->transport == SSL_TRANSPORT_DATAGRAM )\r\n+    {\r\n+        /* Drop unexpected ChangeCipherSpec messages */\r\n+        if( ssl->in_msgtype == SSL_MSG_CHANGE_CIPHER_SPEC &&\r\n+            ssl->state != SSL_CLIENT_CHANGE_CIPHER_SPEC &&\r\n+            ssl->state != SSL_SERVER_CHANGE_CIPHER_SPEC )\r\n+        {\r\n+            SSL_DEBUG_MSG( 1, ( \"dropping unexpected ChangeCipherSpec\" ) );\r\n+            return( POLARSSL_ERR_SSL_INVALID_RECORD );\r\n+        }\r\n+\r\n+        /* Drop unexpected ApplicationData records,\r\n+         * except at the beginning of renegotiations */\r\n+        if( ssl->in_msgtype == SSL_MSG_APPLICATION_DATA &&\r\n+            ssl->state != SSL_HANDSHAKE_OVER &&\r\n+            ! ( ssl->renegotiation == SSL_RENEGOTIATION &&\r\n+                ssl->state == SSL_SERVER_HELLO ) )\r\n+        {\r\n+            SSL_DEBUG_MSG( 1, ( \"dropping unexpected ApplicationData\" ) );\r\n+            return( POLARSSL_ERR_SSL_INVALID_RECORD );\r\n+        }\r\n+    }\r\n #endif /* POLARSSL_SSL_PROTO_DTLS */\r\n \r\n     /* Check length against the size of our buffer */\r\n@@ -3259,8 +3258,12 @@ read_record_header:\r\n             /* Ignore bad record and get next one; drop the whole datagram\r\n              * since current header cannot be trusted to find the next record\r\n              * in current datagram */\r\n-            ssl->next_record_offset = 0;\r\n-            ssl->in_left = 0;\r\n+            if (ret != POLARSSL_ERR_SSL_INVALID_RECORD_EPOCH) { /* Do not drop the whole datagramm in case of wrong epoch */\r\n+                ssl->next_record_offset = 0;\r\n+                ssl->in_left = 0;\r\n+\t    } else { /* just go to the next record */\r\n+                ssl->next_record_offset = ssl->in_msglen + ssl_hdr_len( ssl );\r\n+\t    }\r\n \r\n             SSL_DEBUG_MSG( 1, ( \"discarding invalid record (header)\" ) );\r\n             goto read_record_header;\r\n-- \r\n2.1.4\r\n```\r\n"},"comment":{"url":"https://api.github.com/repos/ARMmbed/mbedtls/issues/comments/165092125","html_url":"https://github.com/ARMmbed/mbedtls/issues/345#issuecomment-165092125","issue_url":"https://api.github.com/repos/ARMmbed/mbedtls/issues/345","id":165092125,"user":{"login":"jeannotlapin","id":10604519,"avatar_url":"https://avatars.githubusercontent.com/u/10604519?v=3","gravatar_id":"","url":"https://api.github.com/users/jeannotlapin","html_url":"https://github.com/jeannotlapin","followers_url":"https://api.github.com/users/jeannotlapin/followers","following_url":"https://api.github.com/users/jeannotlapin/following{/other_user}","gists_url":"https://api.github.com/users/jeannotlapin/gists{/gist_id}","starred_url":"https://api.github.com/users/jeannotlapin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jeannotlapin/subscriptions","organizations_url":"https://api.github.com/users/jeannotlapin/orgs","repos_url":"https://api.github.com/users/jeannotlapin/repos","events_url":"https://api.github.com/users/jeannotlapin/events{/privacy}","received_events_url":"https://api.github.com/users/jeannotlapin/received_events","type":"User","site_admin":false},"created_at":"2015-12-16T12:30:34Z","updated_at":"2015-12-16T12:30:34Z","body":"Hi Simon,\r\nwe're still stuck to polarssl v1.4 in linphone because of DTLS-SRTP. I'm working on migration to mbedTLS (with the patch I provided to enable RFC5764) but not there yet. I won't be able to test your fix before I complete the migration, not before January I'm afraid."}}