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

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

diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c
index 68b4b6bb1ec0814d394d1753b2baf8f87b449b87..cb3a6f9c1ac0bc93c6e18020292948fe4305d7a6 100644 (file)
--- 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;
 

diff --git a/epan/dissectors/packet-ssl-utils.h b/epan/dissectors/packet-ssl-utils.h
index 424ee2168233a867490b7b1f17d121e4ac7b9ef0..df148a98a57617078f5b8c5a5a7b0a2d8f304181 100644 (file)
--- a/epan/dissectors/packet-ssl-utils.h
+++ b/epan/dissectors/packet-ssl-utils.h
@@ -220,8 +220,9 @@ typedef struct _StringInfo {
 #define SSL_MASTER_SECRET       (1<<5)
 #define SSL_PRE_MASTER_SECRET   (1<<6)
 
-#define SSL_CIPHER_MODE_STREAM  0
-#define SSL_CIPHER_MODE_CBC     1
+#define SSL_CIPHER_MODE_STREAM  0 /* GenericStreamCipher */
+#define SSL_CIPHER_MODE_CBC     1 /* GenericBlockCipher */
+#define SSL_CIPHER_MODE_GCM     2 /* GenericAEADCipher */
 
 #define SSL_DEBUG_USE_STDERR "-"