Bug#16218104: MYSQL YASSL - LUCKY THIRTEEN: BREAKING THE

              TLS AND DTLS RECORD PROTOCOLS

Description: In yassl, decryption phase in TLS protocol
             depends on type of padding. This patch
             removes this dependancy and makes error
             generation/decryption process independent
             of padding type.

extra/yassl/include/yassl_error.hpp

@@ -53,7 +53,8 @@ enum YasslError {
     badVersion_error    = 117,
     compress_error      = 118,
     decompress_error    = 119,
-    pms_version_error   = 120
+    pms_version_error   = 120,
+    sanityCipher_error  = 121
 
     // !!!! add error message to .cpp !!!!
 

extra/yassl/include/yassl_types.hpp

@@ -220,7 +220,11 @@ const int DEFAULT_TIMEOUT   = 500;  // Default Session timeout in seconds
 const int MAX_RECORD_SIZE   = 16384; // 2^14, max size by standard
 const int COMPRESS_EXTRA    = 1024;  // extra compression possible addition
 const int SESSION_FLUSH_COUNT = 256;  // when to flush session cache
-
+const int MAX_PAD_SIZE        = 256;  // max TLS padding size
+const int COMPRESS_CONSTANT   =  13;  // compression calculation constant
+const int COMPRESS_UPPER      =  55;  // compression calculation numerator
+const int COMPRESS_LOWER      =  64;  // compression calculation denominator
+const int COMPRESS_DUMMY_SIZE =  64;  // compression dummy round size 
 
 typedef uint8 Cipher;             // first byte is always 0x00 for SSLv3 & TLS
 

extra/yassl/src/handshake.cpp

@@ -221,12 +221,45 @@ void buildSHA(SSL& ssl, Finished& fin, const opaque* sender)
 }
 
 
+// sanity checks on encrypted message size
+static int sanity_check_message(SSL& ssl, uint msgSz)
+{
+    uint minSz = 0;
+
+    if (ssl.getSecurity().get_parms().cipher_type_ == block) {
+        uint blockSz = ssl.getCrypto().get_cipher().get_blockSize();
+        if (msgSz % blockSz)
+            return -1;
+
+        minSz = ssl.getSecurity().get_parms().hash_size_ + 1;  // pad byte too
+        if (blockSz > minSz)
+            minSz = blockSz;
+
+        if (ssl.isTLSv1_1())
+            minSz += blockSz;   // explicit IV
+    }
+    else {      // stream
+        minSz = ssl.getSecurity().get_parms().hash_size_;
+    }
+
+    if (msgSz < minSz)
+        return -1;
+
+    return 0;
+}
+
+
 // decrypt input message in place, store size in case needed later
 void decrypt_message(SSL& ssl, input_buffer& input, uint sz)
 {
     input_buffer plain(sz);
     opaque*      cipher = input.get_buffer() + input.get_current();
 
+    if (sanity_check_message(ssl, sz) != 0) {
+        ssl.SetError(sanityCipher_error);
+        return;
+    }
+
     ssl.useCrypto().use_cipher().decrypt(plain.get_buffer(), cipher, sz);
     memcpy(cipher, plain.get_buffer(), sz);
     ssl.useSecurity().use_parms().encrypt_size_ = sz;
@@ -774,6 +807,8 @@ int DoProcessReply(SSL& ssl)
                     return 0;
                 }
                 decrypt_message(ssl, buffer, hdr.length_);
+                if (ssl.GetError())
+                    return 0;
             }
                 
             mySTL::auto_ptr<Message> msg(mf.CreateObject(hdr.type_));

extra/yassl/src/yassl_error.cpp

@@ -144,6 +144,10 @@ void SetErrorString(YasslError error, char* buffer)
         strncpy(buffer, "bad PreMasterSecret version error", max);
         break;
 
+    case sanityCipher_error :
+        strncpy(buffer, "sanity check on cipher text size error", max);
+        break;
+
         // openssl errors
     case SSL_ERROR_WANT_READ :
         strncpy(buffer, "the read operation would block", max);

extra/yassl/src/yassl_imp.cpp

@@ -972,30 +972,193 @@ output_buffer& operator<<(output_buffer& output, const Data& data)
 }
 
 
+// check all bytes for equality 
+static int constant_compare(const byte* a, const byte* b, int len)
+{
+    int good = 0;
+    int bad  = 0;
+
+    for (int i = 0; i < len; i++) {
+        if (a[i] == b[i])
+            good++;
+        else
+            bad++;
+    }
+
+    if (good == len)
+        return 0;
+    else
+        return 0 - bad;  // failure
+}
+
+
+// check bytes for pad value
+static int pad_check(const byte* input, byte pad, int len)
+{
+    int good = 0;
+    int bad  = 0;
+
+    for (int i = 0; i < len; i++) {
+        if (input[i] == pad)
+            good++;
+        else
+            bad++;
+    }
+
+    if (good == len)
+        return 0;
+    else
+        return 0 - bad;  // failure
+}
+
+
+// get number of compression rounds
+static inline int get_rounds(int pLen, int padLen, int t)
+{
+    int  roundL1 = 1;  // round ups 
+    int  roundL2 = 1;
+
+    int L1 = COMPRESS_CONSTANT + pLen - t;
+    int L2 = COMPRESS_CONSTANT + pLen - padLen - 1 - t;
+
+    L1 -= COMPRESS_UPPER;
+    L2 -= COMPRESS_UPPER;
+
+    if ( (L1 % COMPRESS_LOWER) == 0)
+        roundL1 = 0;
+    if ( (L2 % COMPRESS_LOWER) == 0)
+        roundL2 = 0;
+
+    L1 /= COMPRESS_LOWER;
+    L2 /= COMPRESS_LOWER;
+
+    L1 += roundL1;
+    L2 += roundL2;
+
+    return L1 - L2;
+}
+
+
+// do compression rounds on dummy data
+static inline void compress_rounds(SSL& ssl, int rounds, const byte* dummy)
+{
+    if (rounds) {
+        Digest* digest = NULL;
+
+        MACAlgorithm ma = ssl.getSecurity().get_parms().mac_algorithm_;
+        if (ma == sha) 
+            digest = NEW_YS SHA;
+        else if (ma == md5)
+            digest = NEW_YS MD5;
+        else if (ma == rmd)
+            digest = NEW_YS RMD;
+        else
+            return;
+
+        for (int i = 0; i < rounds; i++)
+            digest->update(dummy, COMPRESS_LOWER);
+
+        ysDelete(digest);    
+    }
+}
+
+
+// timing resistant pad verification
+static int timing_verify(SSL& ssl, const byte* input, int padLen, int t,
+                         int pLen)
+{
+    byte verify[SHA_LEN];
+    byte dummy[MAX_PAD_SIZE];
+
+    memset(dummy, 1, sizeof(dummy));
+
+    if ( (t + padLen + 1) > pLen) {
+        pad_check(dummy, (byte)padLen, MAX_PAD_SIZE);
+        if (ssl.isTLS())
+            TLS_hmac(ssl, verify, input, pLen - t, application_data, 1);
+        else
+            hmac(ssl, verify, input, pLen - t, application_data, 1);
+        constant_compare(verify, input + pLen - t, t);
+
+        return -1;
+    }
+
+    if (pad_check(input + pLen - (padLen + 1), (byte)padLen, padLen + 1) != 0) {
+        pad_check(dummy, (byte)padLen, MAX_PAD_SIZE - padLen - 1);
+        if (ssl.isTLS())
+            TLS_hmac(ssl, verify, input, pLen - t, application_data, 1);
+        else
+            hmac(ssl, verify, input, pLen - t, application_data, 1);
+        constant_compare(verify, input + pLen - t, t);
+
+        return -1;
+    }
+
+    pad_check(dummy, (byte)padLen, MAX_PAD_SIZE - padLen - 1);
+    if (ssl.isTLS())
+        TLS_hmac(ssl, verify, input, pLen - padLen - 1 - t, application_data,1);
+    else
+        hmac(ssl, verify, input, pLen - padLen - 1 - t, application_data, 1);
+
+    compress_rounds(ssl, get_rounds(pLen, padLen, t), dummy);
+
+    if (constant_compare(verify, input + (pLen - padLen - 1 - t), t) != 0)
+        return -1;
+
+    return 0;
+}
+
+
 // Process handler for Data
 void Data::Process(input_buffer& input, SSL& ssl)
 {
     int msgSz = ssl.getSecurity().get_parms().encrypt_size_;
     int pad   = 0, padSz = 0;
     int ivExtra = 0;
+    int digestSz = ssl.getCrypto().get_digest().get_digestSize();
+    const byte* rawData = input.get_buffer() + input.get_current();
+    opaque verify[SHA_LEN];
 
     if (ssl.getSecurity().get_parms().cipher_type_ == block) {
         if (ssl.isTLSv1_1())  // IV
             ivExtra = ssl.getCrypto().get_cipher().get_blockSize();
         pad = *(input.get_buffer() + input.get_current() + msgSz -ivExtra - 1);
         padSz = 1;
+
+        if (ssl.isTLS()) {
+            if (timing_verify(ssl, rawData, pad,digestSz, msgSz-ivExtra) != 0) {
+                ssl.SetError(verify_error);
+                return;
             }
-    int digestSz = ssl.getCrypto().get_digest().get_digestSize();
+        }
+        else {   // SSLv3, some don't do this padding right
+            int sz3 = msgSz - digestSz - pad - 1; 
+            hmac(ssl, verify, rawData, sz3, application_data, true);
+            if (constant_compare(verify, rawData + sz3, digestSz) != 0) {
+                ssl.SetError(verify_error);
+                return;
+            }
+        } 
+    }
+    else {  // stream
+        int streamSz = msgSz - digestSz; 
+        if (ssl.isTLS())
+            TLS_hmac(ssl, verify, rawData, streamSz, application_data, true);
+        else
+            hmac(ssl, verify, rawData, streamSz, application_data, true);
+        if (constant_compare(verify, rawData + streamSz, digestSz) != 0) {
+            ssl.SetError(verify_error);
+            return;
+        }
+    }
+
     int dataSz = msgSz - ivExtra - digestSz - pad - padSz;
-    opaque verify[SHA_LEN];
 
     if (dataSz < 0) {
         ssl.SetError(bad_input);
         return;
     }
 
-    const byte* rawData = input.get_buffer() + input.get_current();
-
     // read data
     if (dataSz) {                               // could be compressed
         if (ssl.CompressionOn()) {
@@ -1013,27 +1176,10 @@ void Data::Process(input_buffer& input, SSL& ssl)
             input.read(data->get_buffer(), dataSz);
             data->add_size(dataSz);
         }
-
-        if (ssl.isTLS())
-            TLS_hmac(ssl, verify, rawData, dataSz, application_data, true);
-        else
-            hmac(ssl, verify, rawData, dataSz, application_data, true);
     }
 
-    // read mac and skip fill
-    opaque mac[SHA_LEN];
-    input.read(mac, digestSz);
-    input.set_current(input.get_current() + pad + padSz);
-
-    // verify
-    if (dataSz) {
-        if (memcmp(mac, verify, digestSz)) {
-            ssl.SetError(verify_error);
-            return;
-        }
-    }
-    else 
-        ssl.get_SEQIncrement(true);  // even though no data, increment verify
+    // advance past mac and fill
+    input.set_current(input.get_current() + digestSz + pad + padSz);
 }