Fixed timing difference resulting from badly formatted padding.

ChangeLog

@@ -4,6 +4,10 @@ PolarSSL ChangeLog
 Changes
    * Allow enabling of dummy error_strerror() to support some use-cases
 
+Security
+   * Removed timing differences during SSL message decryption in
+     ssl_decrypt_buf() due to badly formatted padding
+
 = Version 1.2.4 released 2013-01-25
 Changes
    * Added ssl_handshake_step() to allow single stepping the handshake process

library/ssl_tls.c

@@ -1196,7 +1196,7 @@ static int ssl_encrypt_buf( ssl_context *ssl )
 
 static int ssl_decrypt_buf( ssl_context *ssl )
 {
-    size_t i, padlen;
+    size_t i, padlen = 0, correct = 1;
     unsigned char tmp[POLARSSL_SSL_MAX_MAC_SIZE];
 
     SSL_DEBUG_MSG( 2, ( "=> decrypt buf" ) );
@@ -1211,7 +1211,6 @@ static int ssl_decrypt_buf( ssl_context *ssl )
     if( ssl->transform_in->ivlen == 0 )
     {
 #if defined(POLARSSL_ARC4_C)
-        padlen = 0;
         if( ssl->session_in->ciphersuite == TLS_RSA_WITH_RC4_128_MD5 ||
             ssl->session_in->ciphersuite == TLS_RSA_WITH_RC4_128_SHA )
         {
@@ -1237,8 +1236,6 @@ static int ssl_decrypt_buf( ssl_context *ssl )
         unsigned char add_data[13];
         int ret = POLARSSL_ERR_SSL_FEATURE_UNAVAILABLE;
 
-        padlen = 0;
-
 #if defined(POLARSSL_AES_C) && defined(POLARSSL_GCM_C)
         if( ssl->session_in->ciphersuite == TLS_RSA_WITH_AES_128_GCM_SHA256 ||
             ssl->session_in->ciphersuite == TLS_RSA_WITH_AES_256_GCM_SHA384 ||
@@ -1296,12 +1293,16 @@ static int ssl_decrypt_buf( ssl_context *ssl )
     }
     else
     {
+        /*
+         * Decrypt and check the padding
+         */
         unsigned char *dec_msg;
         unsigned char *dec_msg_result;
         size_t dec_msglen;
+        size_t minlen = 0, fake_padlen;
 
         /*
-         * Decrypt and check the padding
+         * Check immediate ciphertext sanity
          */
         if( ssl->in_msglen % ssl->transform_in->ivlen != 0 )
         {
@@ -1310,6 +1311,17 @@ static int ssl_decrypt_buf( ssl_context *ssl )
             return( POLARSSL_ERR_SSL_INVALID_MAC );
         }
 
+        if( ssl->minor_ver >= SSL_MINOR_VERSION_2 )
+            minlen += ssl->transform_in->ivlen;
+
+        if( ssl->in_msglen < minlen + ssl->transform_in->ivlen ||
+            ssl->in_msglen < minlen + ssl->transform_in->maclen + 1 )
+        {
+            SSL_DEBUG_MSG( 1, ( "msglen (%d) < max( ivlen(%d), maclen (%d) + 1 ) ( + expl IV )",
+                           ssl->in_msglen, ssl->transform_in->ivlen, ssl->transform_in->maclen ) );
+            return( POLARSSL_ERR_SSL_INVALID_MAC );
+        }
+
         dec_msglen = ssl->in_msglen;
         dec_msg = ssl->in_msg;
         dec_msg_result = ssl->in_msg;
@@ -1387,6 +1399,17 @@ static int ssl_decrypt_buf( ssl_context *ssl )
         }
 
         padlen = 1 + ssl->in_msg[ssl->in_msglen - 1];
+        fake_padlen = 256 - padlen;
+
+        if( ssl->in_msglen < ssl->transform_in->maclen + padlen )
+        {
+            SSL_DEBUG_MSG( 1, ( "msglen (%d) < maclen (%d) + padlen (%d)",
+                        ssl->in_msglen, ssl->transform_in->maclen, padlen ) );
+
+            padlen = 0;
+            fake_padlen = 256;
+            correct = 0;
+        }
 
         if( ssl->minor_ver == SSL_MINOR_VERSION_0 )
         {
@@ -1395,24 +1418,33 @@ static int ssl_decrypt_buf( ssl_context *ssl )
                 SSL_DEBUG_MSG( 1, ( "bad padding length: is %d, "
                                     "should be no more than %d",
                                padlen, ssl->transform_in->ivlen ) );
-                padlen = 0;
+                correct = 0;
             }
         }
         else
         {
             /*
-             * TLSv1: always check the padding
+             * TLSv1+: always check the padding up to the first failure
+             * and fake check up to 256 bytes of padding
              */
             for( i = 1; i <= padlen; i++ )
             {
                 if( ssl->in_msg[ssl->in_msglen - i] != padlen - 1 )
                 {
-                    SSL_DEBUG_MSG( 1, ( "bad padding byte: should be "
+                    correct = 0;
-                                        "%02x, but is %02x", padlen - 1,
+                    fake_padlen = 256 - i;
-                                   ssl->in_msg[ssl->in_msglen - i] ) );
                     padlen = 0;
                 }
             }
+            for( i = 1; i <= fake_padlen; i++ )
+            {
+                if( ssl->in_msg[i + 1] != fake_padlen - 1 )
+                    minlen = 0;
+                else
+                    minlen = 1;
+            }
+            if( padlen > 0 && correct == 0)
+                SSL_DEBUG_MSG( 1, ( "bad padding byte detected" ) );
         }
     }
 
@@ -1422,19 +1454,12 @@ static int ssl_decrypt_buf( ssl_context *ssl )
     /*
      * Always compute the MAC (RFC4346, CBCTIME).
      */
-    if( ssl->in_msglen < ssl->transform_in->maclen + padlen )
-    {
-        SSL_DEBUG_MSG( 1, ( "msglen (%d) < maclen (%d) + padlen (%d)",
-                    ssl->in_msglen, ssl->transform_in->maclen, padlen ) );
-        return( POLARSSL_ERR_SSL_INVALID_MAC );
-    }
-
     ssl->in_msglen -= ( ssl->transform_in->maclen + padlen );
 
     ssl->in_hdr[3] = (unsigned char)( ssl->in_msglen >> 8 );
     ssl->in_hdr[4] = (unsigned char)( ssl->in_msglen      );
 
-    memcpy( tmp, ssl->in_msg + ssl->in_msglen, POLARSSL_SSL_MAX_MAC_SIZE );
+    memcpy( tmp, ssl->in_msg + ssl->in_msglen, ssl->transform_in->maclen );
 
     if( ssl->minor_ver == SSL_MINOR_VERSION_0 )
     {
@@ -1459,6 +1484,10 @@ static int ssl_decrypt_buf( ssl_context *ssl )
     }
     else
     {
+        /*
+         * Process MAC and always update for padlen afterwards to make
+         * total time independent of padlen
+         */
         if( ssl->transform_in->maclen == 16 )
              md5_hmac( ssl->transform_in->mac_dec, 16,
                        ssl->in_ctr,  ssl->in_msglen + 13,
@@ -1487,15 +1516,13 @@ static int ssl_decrypt_buf( ssl_context *ssl )
                      ssl->transform_in->maclen ) != 0 )
     {
         SSL_DEBUG_MSG( 1, ( "message mac does not match" ) );
-        return( POLARSSL_ERR_SSL_INVALID_MAC );
+        correct = 0;
     }
 
     /*
-     * Finally check the padding length; bad padding
+     * Finally check the correct flag
-     * will produce the same error as an invalid MAC.
      */
-    if( ssl->transform_in->ivlen != 0 && ssl->transform_in->ivlen != 12 &&
+    if( correct == 0 )
-        padlen == 0 )
         return( POLARSSL_ERR_SSL_INVALID_MAC );
 
     if( ssl->in_msglen == 0 )