Merge remote-tracking branch 'public/pr/1852' into development

ChangeLog

@@ -37,6 +37,16 @@ Bugfix
    * Fix compiler warning of 'use before initialisation' in
      mbedtls_pk_parse_key(). Found by Martin Boye Petersen and fixed by Dawid
      Drozd. #1098
+   * Fix decryption of zero length messages (all padding) in some circumstances:
+     DTLS 1.0 and 1.2, and CBC ciphersuites using encrypt-then-MAC. Most often
+     seen when communicating with OpenSSL using TLS 1.0. Reported by @kFYatek
+     (#1632) and by Conor Murphy on the forum. Fix contributed by Espressif
+     Systems.
+   * Fail when receiving a TLS alert message with an invalid length, or invalid
+     zero-length messages when using TLS 1.2. Contributed by Espressif Systems.
+   * Fix ssl_client2 example to send application data with 0-length content
+     when the request_size argument is set to 0 as stated in the documentation.
+     Fixes #1833.
 
 Changes
    * Change the shebang line in Perl scripts to look up perl in the PATH.

library/ssl_tls.c

@@ -1979,28 +1979,28 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl )
              * and fake check up to 256 bytes of padding
              */
             size_t pad_count = 0, real_count = 1;
-            size_t padding_idx = ssl->in_msglen - padlen - 1;
+            size_t padding_idx = ssl->in_msglen - padlen;
             size_t i;
 
             /*
              * Padding is guaranteed to be incorrect if:
-             *   1. padlen >= ssl->in_msglen
+             *   1. padlen > ssl->in_msglen
              *
-             *   2. padding_idx >= MBEDTLS_SSL_IN_CONTENT_LEN +
+             *   2. padding_idx > MBEDTLS_SSL_IN_CONTENT_LEN +
              *                     ssl->transform_in->maclen
              *
              * In both cases we reset padding_idx to a safe value (0) to
              * prevent out-of-buffer reads.
              */
-            correct &= ( ssl->in_msglen >= padlen + 1 );
+            correct &= ( padlen <= ssl->in_msglen );
-            correct &= ( padding_idx < MBEDTLS_SSL_IN_CONTENT_LEN +
+            correct &= ( padding_idx <= MBEDTLS_SSL_IN_CONTENT_LEN +
                                        ssl->transform_in->maclen );
 
             padding_idx *= correct;
 
-            for( i = 1; i <= 256; i++ )
+            for( i = 0; i < 256; i++ )
             {
-                real_count &= ( i <= padlen );
+                real_count &= ( i < padlen );
                 pad_count += real_count *
                              ( ssl->in_msg[padding_idx + i] == padlen - 1 );
             }
@@ -2133,6 +2133,16 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl )
 
     if( ssl->in_msglen == 0 )
     {
+#if defined(MBEDTLS_SSL_PROTO_TLS1_2)
+        if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3
+            && ssl->in_msgtype != MBEDTLS_SSL_MSG_APPLICATION_DATA )
+        {
+            /* TLS v1.2 explicitly disallows zero-length messages which are not application data */
+            MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid zero-length message type: %d", ssl->in_msgtype ) );
+            return( MBEDTLS_ERR_SSL_INVALID_RECORD );
+        }
+#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */
+
         ssl->nb_zero++;
 
         /*
@@ -4177,6 +4187,16 @@ int mbedtls_ssl_handle_message_type( mbedtls_ssl_context *ssl )
 
     if( ssl->in_msgtype == MBEDTLS_SSL_MSG_ALERT )
     {
+        if( ssl->in_msglen != 2 )
+        {
+            /* Note: Standard allows for more than one 2 byte alert
+               to be packed in a single message, but Mbed TLS doesn't
+               currently support this. */
+            MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid alert message, len: %d",
+                           ssl->in_msglen ) );
+            return( MBEDTLS_ERR_SSL_INVALID_RECORD );
+        }
+
         MBEDTLS_SSL_DEBUG_MSG( 2, ( "got an alert message, type: [%d:%d]",
                        ssl->in_msg[0], ssl->in_msg[1] ) );
 

programs/ssl/ssl_client2.c

@@ -248,6 +248,10 @@ int main( void )
     "    request_page=%%s     default: \".\"\n"             \
     "    request_size=%%d     default: about 34 (basic request)\n"           \
     "                        (minimum: 0, max: " MAX_REQUEST_SIZE_STR ")\n"  \
+    "                        If 0, in the first exchange only an empty\n"    \
+    "                        application data message is sent followed by\n" \
+    "                        a second non-empty message before attempting\n" \
+    "                        to read a response from the server\n"           \
     "    debug_level=%%d      default: 0 (disabled)\n"             \
     "    nbio=%%d             default: 0 (blocking I/O)\n"         \
     "                        options: 1 (non-blocking), 2 (added delays)\n"   \
@@ -1663,10 +1667,13 @@ send_request:
 
     if( opt.transport == MBEDTLS_SSL_TRANSPORT_STREAM )
     {
-        for( written = 0, frags = 0; written < len; written += ret, frags++ )
+        written = 0;
+        frags = 0;
+
+        do
         {
             while( ( ret = mbedtls_ssl_write( &ssl, buf + written,
-                                              len - written ) ) <= 0 )
+                                              len - written ) ) < 0 )
             {
                 if( ret != MBEDTLS_ERR_SSL_WANT_READ &&
                     ret != MBEDTLS_ERR_SSL_WANT_WRITE )
@@ -1686,7 +1693,11 @@ send_request:
 #endif
                 }
             }
+
+            frags++;
+            written += ret;
         }
+        while( written < len );
     }
     else /* Not stream, so datagram */
     {
@@ -1730,6 +1741,13 @@ send_request:
     mbedtls_printf( " %d bytes written in %d fragments\n\n%s\n",
                     written, frags, (char *) buf );
 
+    /* Send a non-empty request if request_size == 0 */
+    if ( len == 0 )
+    {
+        opt.request_size = DFL_REQUEST_SIZE;
+        goto send_request;
+    }
+
     /*
      * 7. Read the HTTP response
      */

tests/ssl-opt.sh

@@ -1170,6 +1170,38 @@ run_test    "Fallback SCSV: enabled, max version, openssl client" \
             -s "received FALLBACK_SCSV" \
             -S "inapropriate fallback"
 
+# Test sending and receiving empty application data records
+
+run_test    "Encrypt then MAC: empty application data record" \
+            "$P_SRV auth_mode=none debug_level=4 etm=1" \
+            "$P_CLI auth_mode=none etm=1 request_size=0 force_ciphersuite=TLS-ECDHE-RSA-WITH-AES-256-CBC-SHA" \
+            0 \
+            -S "0000:  0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f" \
+            -s "dumping 'input payload after decrypt' (0 bytes)" \
+            -c "0 bytes written in 1 fragments"
+
+run_test    "Default, no Encrypt then MAC: empty application data record" \
+            "$P_SRV auth_mode=none debug_level=4 etm=0" \
+            "$P_CLI auth_mode=none etm=0 request_size=0" \
+            0 \
+            -s "dumping 'input payload after decrypt' (0 bytes)" \
+            -c "0 bytes written in 1 fragments"
+
+run_test    "Encrypt then MAC, DTLS: empty application data record" \
+            "$P_SRV auth_mode=none debug_level=4 etm=1 dtls=1" \
+            "$P_CLI auth_mode=none etm=1 request_size=0 force_ciphersuite=TLS-ECDHE-RSA-WITH-AES-256-CBC-SHA dtls=1" \
+            0 \
+            -S "0000:  0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f 0f" \
+            -s "dumping 'input payload after decrypt' (0 bytes)" \
+            -c "0 bytes written in 1 fragments"
+
+run_test    "Default, no Encrypt then MAC, DTLS: empty application data record" \
+            "$P_SRV auth_mode=none debug_level=4 etm=0 dtls=1" \
+            "$P_CLI auth_mode=none etm=0 request_size=0 dtls=1" \
+            0 \
+            -s "dumping 'input payload after decrypt' (0 bytes)" \
+            -c "0 bytes written in 1 fragments"
+
 ## ClientHello generated with
 ## "openssl s_client -CAfile tests/data_files/test-ca.crt -tls1_1 -connect localhost:4433 -cipher ..."
 ## then manually twiddling the ciphersuite list.