From 63eca930d7fac35b073ac2a72f1dfd7cb6f0158d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= <mpg@elzevir.fr>
Date: Mon, 8 Sep 2014 16:39:08 +0200
Subject: [PATCH] Drop invalid records with DTLS

---
 library/ssl_tls.c         | 62 +++++++++++++++++++++++++++++++++------
 programs/test/udp_proxy.c |  4 +--
 tests/ssl-opt.sh          | 11 +++++++
 3 files changed, 65 insertions(+), 12 deletions(-)

diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 4f4fb7b15..7a75815bb 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -2579,14 +2579,6 @@ static int ssl_prepare_record_content( ssl_context *ssl )
     {
         if( ( ret = ssl_decrypt_buf( ssl ) ) != 0 )
         {
-#if defined(POLARSSL_SSL_ALERT_MESSAGES)
-            if( ret == POLARSSL_ERR_SSL_INVALID_MAC )
-            {
-                ssl_send_alert_message( ssl,
-                                        SSL_ALERT_LEVEL_FATAL,
-                                        SSL_ALERT_MSG_BAD_RECORD_MAC );
-            }
-#endif
             SSL_DEBUG_RET( 1, "ssl_decrypt_buf", ret );
             return( ret );
         }
@@ -2620,6 +2612,12 @@ static int ssl_prepare_record_content( ssl_context *ssl )
     return( 0 );
 }
 
+/*
+ * Read a record.
+ *
+ * For DTLS, silently ignore invalid records (RFC 4.1.2.7.)
+ * and continue reading until a valid record is found.
+ */
 int ssl_read_record( ssl_context *ssl )
 {
     int ret;
@@ -2656,6 +2654,9 @@ int ssl_read_record( ssl_context *ssl )
     /*
      * Read the record header and parse it
      */
+#if defined(POLARSSL_SSL_PROTO_DTLS)
+read_record_header:
+#endif
     if( ( ret = ssl_fetch_input( ssl, ssl_hdr_len( ssl ) ) ) != 0 )
     {
         SSL_DEBUG_RET( 1, "ssl_fetch_input", ret );
@@ -2663,7 +2664,22 @@ int ssl_read_record( ssl_context *ssl )
     }
 
     if( ( ret = ssl_parse_record_header( ssl ) ) != 0 )
+    {
+#if defined(POLARSSL_SSL_PROTO_DTLS)
+        if( ssl->transport == SSL_TRANSPORT_DATAGRAM )
+        {
+            /* Ignore bad record and get next one; drop the whole datagram
+             * since current header cannot be trusted to find the next record
+             * in current datagram */
+            ssl->next_record_offset = 0;
+            ssl->in_left = 0;
+
+            SSL_DEBUG_MSG( 2, ( "discarding invalid record" ) );
+            goto read_record_header;
+        }
+#endif
         return( ret );
+    }
 
     /*
      * Read and optionally decrypt the message contents
@@ -2684,7 +2700,35 @@ int ssl_read_record( ssl_context *ssl )
         ssl->in_left = 0;
 
     if( ( ret = ssl_prepare_record_content( ssl ) ) != 0 )
-        return( ret );
+    {
+#if defined(POLARSSL_SSL_PROTO_DTLS)
+        if( ssl->transport == SSL_TRANSPORT_DATAGRAM )
+        {
+            /* Silently discard invalid records */
+            if( ret == POLARSSL_ERR_SSL_INVALID_RECORD ||
+                ret == POLARSSL_ERR_SSL_INVALID_MAC )
+            {
+                SSL_DEBUG_MSG( 2, ( "discarding invalid record" ) );
+                goto read_record_header;
+            }
+
+            return( ret );
+        }
+        else
+#endif
+        {
+            /* Error out (and send alert) on invalid records */
+#if defined(POLARSSL_SSL_ALERT_MESSAGES)
+            if( ret == POLARSSL_ERR_SSL_INVALID_MAC )
+            {
+                ssl_send_alert_message( ssl,
+                        SSL_ALERT_LEVEL_FATAL,
+                        SSL_ALERT_MSG_BAD_RECORD_MAC );
+            }
+#endif
+            return( ret );
+        }
+    }
 
     /*
      * Handle particular types of records
diff --git a/programs/test/udp_proxy.c b/programs/test/udp_proxy.c
index a0ff87341..23eb2ab26 100644
--- a/programs/test/udp_proxy.c
+++ b/programs/test/udp_proxy.c
@@ -297,10 +297,8 @@ int send_packet( const packet *p, const char *why )
     }
 
     /* Don't duplicate Application Data, only handshake covered */
-    // Don't duplicate CSS for now (TODO later)
     if( opt.duplicate != 0 &&
         strcmp( p->type, "ApplicationData" ) != 0 &&
-        strcmp( p->type, "ChangeCipherSpec" ) != 0 &&
         ++dupl_cnt == opt.duplicate )
     {
         dupl_cnt = 0;
@@ -322,7 +320,7 @@ int handle_message( const char *way, int dst, int src )
     packet cur;
     static packet prev;
 
-    /* receivec packet */
+    /* receive packet */
     if( ( ret = net_recv( &src, cur.buf, sizeof( cur.buf ) ) ) <= 0 )
     {
         printf( "  ! net_recv returned %d\n", ret );
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index c7758b883..f375b1b8e 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -2104,6 +2104,7 @@ run_test    "DTLS proxy: reference" \
             "$P_SRV dtls=1" \
             "$P_CLI dtls=1" \
             0 \
+            -s "Extra-header:" \
             -c "HTTP/1.0 200 OK"
 
 run_test    "DTLS proxy: some duplication" \
@@ -2111,6 +2112,7 @@ run_test    "DTLS proxy: some duplication" \
             "$P_SRV dtls=1" \
             "$P_CLI dtls=1" \
             0 \
+            -s "Extra-header:" \
             -c "HTTP/1.0 200 OK"
 
 run_test    "DTLS proxy: lots of duplication" \
@@ -2118,6 +2120,15 @@ run_test    "DTLS proxy: lots of duplication" \
             "$P_SRV dtls=1" \
             "$P_CLI dtls=1" \
             0 \
+            -s "Extra-header:" \
+            -c "HTTP/1.0 200 OK"
+
+run_test    "DTLS proxy: inject invalid AD record" \
+            -p "$P_PXY bad_ad=1" \
+            "$P_SRV dtls=1" \
+            "$P_CLI dtls=1" \
+            0 \
+            -s "Extra-header:" \
             -c "HTTP/1.0 200 OK"
 
 # Final report