CBC mode: Allow zero-length message fragments (100% padding)

Fixes Mbed-TLS#1632
espressif · Jul 20, 2018 · 5556dd2 · 5556dd2
1 parent 7d728bd
commit 5556dd2
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 7 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -34,6 +34,11 @@ Bugfix
      i386 with SSE2. Found by László Langó. Fixes #1550
    * Fix namespacing in header files. Remove the `mbedtls` namespacing in
      the `#include` in the header files. Resolves #857
+   * 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.
 
 Changes
    * Change the shebang line in Perl scripts to look up perl in the PATH.

diff --git a/library/ssl_tls.c b/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 &= ( padding_idx < MBEDTLS_SSL_IN_CONTENT_LEN +
+            correct &= ( padlen <= ssl->in_msglen );
+            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 );
             }