From Peter Wu via https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=9144 [PATCH] Update TLS Ciphers

Use IV from record for CBC mode, add padding/IV length check

Add summary of RFCs to make it more obvious why certain parts (IV, MAC,
padding) are used. Merge DTLS and TLS blocks for extracting IV. This
saves an unnecessary memmove() because the input pointer is, well, just
a local variable and can therefore be incremented.

Validate padding and IV lengths before using it. A crash could occur
if the explicit IV is missing (this would make memmove write before its
buffer). The missing padding check had as implication that a misleading
error is returning with a negative length (not exploitable).

Use IV from record for CBC mode, previously it decrypted the first block
incorrectly and then threw this "decrypted" IV away. Now it extracts the
IV and uses this for decrypting the first fragment block. (remember that
CBC xor's the output of the block cipher with the previous ciphertext
(or IV for the first block)).

This is a preparation for GCM which does not have a MAC. The skip_mac
branch is necessary to make the compiler happy in this patch, 'mac'
could otherwise be uninitialised.

svn path=/trunk/; revision=52149

1 files changed, 56 insertions, 22 deletions

diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c
index 68b4b6bb1e..cb3a6f9c1a 100644
--- a/epan/dissectors/packet-ssl-utils.c
+++ b/epan/dissectors/packet-ssl-utils.c
@@ -2782,6 +2782,41 @@ ssl_decrypt_record(SslDecryptSession*ssl,SslDecoder* decoder, gint ct,
         ssl_data_realloc(out_str, inl + 32);
     }
 
+    /* RFC 6101/2246: SSLCipherText/TLSCipherText has two structures for types:
+     * (notation: { unencrypted, [ encrypted ] })
+     * GenericStreamCipher: { [content, mac] }
+     * GenericBlockCipher: { IV (TLS 1.1+), [content, mac, padding, padding_len] }
+     * RFC 5426 (TLS 1.2): TLSCipherText has additionally:
+     * GenericAEADCipher: { nonce_explicit, [content] }
+     * RFC 4347 (DTLS): based on TLS 1.1, only GenericBlockCipher is supported.
+     * RFC 6347 (DTLS 1.2): based on TLS 1.2, includes GenericAEADCipher too.
+     */
+
+    /* (TLS 1.1 and later, DTLS) Extract explicit IV for GenericBlockCipher */
+    if (decoder->cipher_suite->mode == SSL_CIPHER_MODE_CBC) {
+        switch (ssl->version_netorder) {
+        case TLSV1DOT1_VERSION:
+        case TLSV1DOT2_VERSION:
+        case DTLSV1DOT0_VERSION:
+        case DTLSV1DOT2_VERSION:
+        case DTLSV1DOT0_VERSION_NOT:
+            if ((gint)inl < decoder->cipher_suite->block) {
+                ssl_debug_printf("ssl_decrypt_record failed: input %d has no space for IV %d\n",
+                        inl, decoder->cipher_suite->block);
+                return -1;
+            }
+            pad = gcry_cipher_setiv(decoder->evp, in, decoder->cipher_suite->block);
+            if (pad != 0) {
+                ssl_debug_printf("ssl_decrypt_record failed: failed to set IV: %s %s\n",
+                        gcry_strsource (pad), gcry_strerror (pad));
+            }
+
+            inl -= decoder->cipher_suite->block;
+            in += decoder->cipher_suite->block;
+            break;
+        }
+    }
+
     /* First decrypt*/
     if ((pad = ssl_cipher_decrypt(&decoder->evp, out_str->data, out_str->data_len, in, inl))!= 0) {
         ssl_debug_printf("ssl_decrypt_record failed: ssl_cipher_decrypt: %s %s\n", gcry_strsource (pad),
@@ -2792,35 +2827,33 @@ ssl_decrypt_record(SslDecryptSession*ssl,SslDecoder* decoder, gint ct,
     ssl_print_data("Plaintext", out_str->data, inl);
     worklen=inl;
 
-    /* Now strip off the padding*/
-    if(decoder->cipher_suite->block!=1) {
+    /* strip padding for GenericBlockCipher */
+    if (decoder->cipher_suite->mode == SSL_CIPHER_MODE_CBC) {
         pad=out_str->data[inl-1];
+        if (worklen <= pad) {
+            ssl_debug_printf("ssl_decrypt_record failed: padding %d too large for work %d\n",
+                pad, worklen);
+            return -1;
+        }
         worklen-=(pad+1);
         ssl_debug_printf("ssl_decrypt_record found padding %d final len %d\n",
             pad, worklen);
     }
 
-    /* And the MAC */
-    if (ssl_cipher_suite_dig(decoder->cipher_suite)->len > (gint)worklen)
-    {
-        ssl_debug_printf("ssl_decrypt_record wrong record len/padding outlen %d\n work %d\n",*outl, worklen);
-        return -1;
-    }
-    worklen-=ssl_cipher_suite_dig(decoder->cipher_suite)->len;
-    mac = out_str->data + worklen;
-
-    /* if TLS 1.1 or 1.2 we use the transmitted IV and remove it after (to not modify dissector in others parts)*/
-    if(ssl->version_netorder==TLSV1DOT1_VERSION || ssl->version_netorder==TLSV1DOT2_VERSION){
-        /* if stream cipher used, IV is not contained */
-        worklen=worklen-(decoder->cipher_suite->block!=1 ? decoder->cipher_suite->block : 0);
-        memmove(out_str->data,out_str->data+(decoder->cipher_suite->block!=1 ? decoder->cipher_suite->block : 0),worklen);
-    }
-    if(ssl->version_netorder==DTLSV1DOT0_VERSION ||
-      ssl->version_netorder==DTLSV1DOT2_VERSION ||
-      ssl->version_netorder==DTLSV1DOT0_VERSION_NOT){
-        worklen=worklen-decoder->cipher_suite->block;
-        memmove(out_str->data,out_str->data+decoder->cipher_suite->block,worklen);
+    /* MAC for GenericStreamCipher and GenericBlockCipher */
+    if (decoder->cipher_suite->mode == SSL_CIPHER_MODE_STREAM ||
+        decoder->cipher_suite->mode == SSL_CIPHER_MODE_CBC) {
+        if (ssl_cipher_suite_dig(decoder->cipher_suite)->len > (gint)worklen) {
+            ssl_debug_printf("ssl_decrypt_record wrong record len/padding outlen %d\n work %d\n",*outl, worklen);
+            return -1;
+        }
+        worklen-=ssl_cipher_suite_dig(decoder->cipher_suite)->len;
+        mac = out_str->data + worklen;
+    } else /* if (decoder->cipher_suite->mode == SSL_CIPHER_MODE_GCM) */ {
+        /* GenericAEADCipher has no MAC */
+        goto skip_mac;
     }
+
     /* Now check the MAC */
     ssl_debug_printf("checking mac (len %d, version %X, ct %d seq %d)\n",
         worklen, ssl->version_netorder, ct, decoder->seq);
@@ -2870,6 +2903,7 @@ ssl_decrypt_record(SslDecryptSession*ssl,SslDecoder* decoder, gint ct,
             return -1;
         }
     }
+skip_mac:
 
     *outl = worklen;