osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

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
This commit is contained in:
Alexis La Goutte 2013-09-19 20:27:05 +00:00
parent cf7f5dd3ad
commit a87da76132
2 changed files with 58 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

76
epan/dissectors/packet-ssl-utils.c
View File

@ -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;
/* 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;
}
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);
}
/* 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;

5
epan/dissectors/packet-ssl-utils.h
View File

@ -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 "-"