From 37ae952923cd411ae387127184904aab773c8225 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Fri, 3 May 2019 16:54:26 +0100
Subject: [PATCH] Move dropping of unexpected AD records to after record
 decryption

With the introduction of the CID extension, the record content type
may change during decryption; we must therefore re-consider every
record content type check that happens before decryption, and either
move or duplicate it to ensure it also applies to records whose
real content type is only revealed during decryption.

This commit does this for the silent dropping of unexpected
ApplicationData records in DTLS. Previously, this was caught
in ssl_parse_record_header(), returning
MBEDTLS_ERR_SSL_UNEXPECTED_RECORD which in ssl_get_next_record()
would lead to silent skipping of the record.

When using CID, this check wouldn't trigger e.g. when delayed
encrypted ApplicationData records come on a CID-based connection
during a renegotiation.

This commit moves the check to mbedtls_ssl_handle_message_type()
and returns MBEDTLS_ERR_SSL_NON_FATAL if it triggers, which leads
so silent skipover in the caller mbedtls_ssl_read_record().
---
 library/ssl_tls.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index dacbde6d5..1192d24d9 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -4932,20 +4932,6 @@ static int ssl_parse_record_header( mbedtls_ssl_context *ssl )
             return( MBEDTLS_ERR_SSL_UNEXPECTED_RECORD );
         }
 #endif
-
-        /* Drop unexpected ApplicationData records,
-         * except at the beginning of renegotiations */
-        if( ssl->in_msgtype == MBEDTLS_SSL_MSG_APPLICATION_DATA &&
-            ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER
-#if defined(MBEDTLS_SSL_RENEGOTIATION)
-            && ! ( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS &&
-                   ssl->state == MBEDTLS_SSL_SERVER_HELLO )
-#endif
-            )
-        {
-            MBEDTLS_SSL_DEBUG_MSG( 1, ( "dropping unexpected ApplicationData" ) );
-            return( MBEDTLS_ERR_SSL_UNEXPECTED_RECORD );
-        }
     }
 #endif /* MBEDTLS_SSL_PROTO_DTLS */
 
@@ -6054,13 +6040,29 @@ int mbedtls_ssl_handle_message_type( mbedtls_ssl_context *ssl )
     }
 
 #if defined(MBEDTLS_SSL_PROTO_DTLS)
-    if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM &&
-        ssl->handshake != NULL &&
-        ssl->state == MBEDTLS_SSL_HANDSHAKE_OVER  )
+    if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM )
     {
-        ssl_handshake_wrapup_free_hs_transform( ssl );
-    }
+        /* Drop unexpected ApplicationData records,
+         * except at the beginning of renegotiations */
+        if( ssl->in_msgtype == MBEDTLS_SSL_MSG_APPLICATION_DATA &&
+            ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER
+#if defined(MBEDTLS_SSL_RENEGOTIATION)
+            && ! ( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS &&
+                   ssl->state == MBEDTLS_SSL_SERVER_HELLO )
 #endif
+            )
+        {
+            MBEDTLS_SSL_DEBUG_MSG( 1, ( "dropping unexpected ApplicationData" ) );
+            return( MBEDTLS_ERR_SSL_NON_FATAL );
+        }
+
+        if( ssl->handshake != NULL &&
+            ssl->state == MBEDTLS_SSL_HANDSHAKE_OVER  )
+        {
+            ssl_handshake_wrapup_free_hs_transform( ssl );
+        }
+    }
+#endif /* MBEDTLS_SSL_DTLS */
 
     return( 0 );
 }