Wireshark-dev: [Wireshark-dev] [patch] sparse fix to SSL decryption code
From: Paolo Abeni <paolo.abeni@xxxxxxxx>
Date: Fri, 04 Jul 2008 09:02:06 +0200
hello, the attached patch fix some glitches in the ssl decryption code, namely: - the StringInfo allocator is allowed to return a NULL pointer if the requested data length is 0 - the cipher_suites table is expanded and many wrong values are now fixed - some duplicate code about ssl session state checking is unified into the proper location - the ssl session structure is now properly initialize (a couple of fields where manged in the wrong way) - added some more verbose debug messages I tested the new code against the pcap trace available on the web site and it work properly. The patch is against svn revision 25668. cheers, Paolo -- Email.it, the professional e-mail, gratis per te: http://www.email.it/f Sponsor: VOGLIA DI VACANZE ? * I Costahotels sono gli alberghi specializzati in tour enogastronomici nell'entroterra Romagnolo. Rimini, Riccione, Misano. Clicca qui: http://adv.email.it/cgi-bin/foclick.cgi?mid=8034&d=4-7
Index: epan/dissectors/packet-ssl-utils.c
===================================================================
--- epan/dissectors/packet-ssl-utils.c (revision 25665)
+++ epan/dissectors/packet-ssl-utils.c (working copy)
@@ -549,7 +549,9 @@
ssl_data_alloc(StringInfo* str, guint len)
{
str->data = g_malloc(len);
- if (!str->data)
+ /* the allocator can return a null pointer for a size equal to 0, and that
+ * must be allowed */
+ if (len > 0 && !str->data)
return -1;
str->data_len = len;
return 0;
@@ -936,29 +938,30 @@
{8,KEX_RSA,SIG_RSA,ENC_DES,8,64,40,DIG_SHA,20,1, SSL_CIPHER_MODE_CBC},
{9,KEX_RSA,SIG_RSA,ENC_DES,8,64,64,DIG_SHA,20,0, SSL_CIPHER_MODE_CBC},
{10,KEX_RSA,SIG_RSA,ENC_3DES,8,192,192,DIG_SHA,20,0, SSL_CIPHER_MODE_CBC},
- {11,KEX_DH,SIG_DSS,ENC_DES,8,64,40,DIG_SHA,20,1, SSL_CIPHER_MODE_STREAM},
- {12,KEX_DH,SIG_DSS,ENC_DES,8,64,64,DIG_SHA,20,0, SSL_CIPHER_MODE_STREAM},
- {13,KEX_DH,SIG_DSS,ENC_3DES,8,192,192,DIG_SHA,20,0, SSL_CIPHER_MODE_STREAM},
- {14,KEX_DH,SIG_RSA,ENC_DES,8,64,40,DIG_SHA,20,1, SSL_CIPHER_MODE_STREAM},
- {15,KEX_DH,SIG_RSA,ENC_DES,8,64,64,DIG_SHA,20,0, SSL_CIPHER_MODE_STREAM},
- {16,KEX_DH,SIG_RSA,ENC_3DES,8,192,192,DIG_SHA,20,0, SSL_CIPHER_MODE_STREAM},
- {17,KEX_DH,SIG_DSS,ENC_DES,8,64,40,DIG_SHA,20,1, SSL_CIPHER_MODE_STREAM},
- {18,KEX_DH,SIG_DSS,ENC_DES,8,64,64,DIG_SHA,20,0, SSL_CIPHER_MODE_STREAM},
- {19,KEX_DH,SIG_DSS,ENC_3DES,8,192,192,DIG_SHA,20,0, SSL_CIPHER_MODE_STREAM},
- {20,KEX_DH,SIG_RSA,ENC_DES,8,64,40,DIG_SHA,20,1, SSL_CIPHER_MODE_STREAM},
- {21,KEX_DH,SIG_RSA,ENC_DES,8,64,64,DIG_SHA,20,0, SSL_CIPHER_MODE_STREAM},
- {22,KEX_DH,SIG_RSA,ENC_3DES,8,192,192,DIG_SHA,20,0, SSL_CIPHER_MODE_STREAM},
+ {11,KEX_DH,SIG_DSS,ENC_DES,8,64,40,DIG_SHA,20,1, SSL_CIPHER_MODE_CBC},
+ {12,KEX_DH,SIG_DSS,ENC_DES,8,64,64,DIG_SHA,20,0, SSL_CIPHER_MODE_CBC},
+ {13,KEX_DH,SIG_DSS,ENC_3DES,8,192,192,DIG_SHA,20,0, SSL_CIPHER_MODE_CBC},
+ {14,KEX_DH,SIG_RSA,ENC_DES,8,64,40,DIG_SHA,20,1, SSL_CIPHER_MODE_CBC},
+ {15,KEX_DH,SIG_RSA,ENC_DES,8,64,64,DIG_SHA,20,0, SSL_CIPHER_MODE_CBC},
+ {16,KEX_DH,SIG_RSA,ENC_3DES,8,192,192,DIG_SHA,20,0, SSL_CIPHER_MODE_CBC},
+ {17,KEX_DH,SIG_DSS,ENC_DES,8,64,40,DIG_SHA,20,1, SSL_CIPHER_MODE_CBC},
+ {18,KEX_DH,SIG_DSS,ENC_DES,8,64,64,DIG_SHA,20,0, SSL_CIPHER_MODE_CBC},
+ {19,KEX_DH,SIG_DSS,ENC_3DES,8,192,192,DIG_SHA,20,0, SSL_CIPHER_MODE_CBC},
+ {20,KEX_DH,SIG_RSA,ENC_DES,8,64,40,DIG_SHA,20,1, SSL_CIPHER_MODE_CBC},
+ {21,KEX_DH,SIG_RSA,ENC_DES,8,64,64,DIG_SHA,20,0, SSL_CIPHER_MODE_CBC},
+ {22,KEX_DH,SIG_RSA,ENC_3DES,8,192,192,DIG_SHA,20,0, SSL_CIPHER_MODE_CBC},
{23,KEX_DH,SIG_NONE,ENC_RC4,1,128,40,DIG_MD5,16,1, SSL_CIPHER_MODE_STREAM},
{24,KEX_DH,SIG_NONE,ENC_RC4,1,128,128,DIG_MD5,16,0, SSL_CIPHER_MODE_STREAM},
- {25,KEX_DH,SIG_NONE,ENC_DES,8,64,40,DIG_MD5,16,1, SSL_CIPHER_MODE_STREAM},
- {26,KEX_DH,SIG_NONE,ENC_DES,8,64,64,DIG_MD5,16,0, SSL_CIPHER_MODE_STREAM},
- {27,KEX_DH,SIG_NONE,ENC_3DES,8,192,192,DIG_MD5,16,0, SSL_CIPHER_MODE_STREAM},
+ {25,KEX_DH,SIG_NONE,ENC_DES,8,64,40,DIG_MD5,16,1, SSL_CIPHER_MODE_CBC},
+ {26,KEX_DH,SIG_NONE,ENC_DES,8,64,64,DIG_MD5,16,0, SSL_CIPHER_MODE_CBC},
+ {27,KEX_DH,SIG_NONE,ENC_3DES,8,192,192,DIG_MD5,16,0, SSL_CIPHER_MODE_CBC},
{47,KEX_RSA,SIG_RSA,ENC_AES,16,128,128,DIG_SHA,20,0, SSL_CIPHER_MODE_CBC},
+ {51,KEX_DH, SIG_RSA,ENC_AES,16,128,128,DIG_SHA,20,0, SSL_CIPHER_MODE_CBC},
{53,KEX_RSA,SIG_RSA,ENC_AES256,16,256,256,DIG_SHA,20,0, SSL_CIPHER_MODE_CBC},
{96,KEX_RSA,SIG_RSA,ENC_RC4,1,128,56,DIG_MD5,16,1, SSL_CIPHER_MODE_STREAM},
{97,KEX_RSA,SIG_RSA,ENC_RC2,1,128,56,DIG_MD5,16,1, SSL_CIPHER_MODE_STREAM},
{98,KEX_RSA,SIG_RSA,ENC_DES,8,64,64,DIG_SHA,20,1, SSL_CIPHER_MODE_STREAM},
- {99,KEX_DH,SIG_DSS,ENC_DES,8,64,64,DIG_SHA,16,1, SSL_CIPHER_MODE_STREAM},
+ {99,KEX_DH,SIG_DSS,ENC_DES,8,64,64,DIG_SHA,16,1, SSL_CIPHER_MODE_CBC},
{100,KEX_RSA,SIG_RSA,ENC_RC4,1,128,56,DIG_SHA,20,1, SSL_CIPHER_MODE_STREAM},
{101,KEX_DH,SIG_DSS,ENC_RC4,1,128,56,DIG_SHA,20,1, SSL_CIPHER_MODE_STREAM},
{102,KEX_DH,SIG_DSS,ENC_RC4,1,128,128,DIG_SHA,20,0, SSL_CIPHER_MODE_STREAM},
@@ -1039,12 +1042,19 @@
usage_len = strlen(usage);
/* initalize buffer for sha, md5 random seed*/
- if (ssl_data_alloc(&sha_out, MAX(out->data_len,20)) < 0)
+ if (ssl_data_alloc(&sha_out, MAX(out->data_len,20)) < 0) {
+ ssl_debug_printf("tls_prf: can't allocate sha out\n");
return -1;
- if (ssl_data_alloc(&md5_out, MAX(out->data_len,16)) < 0)
+ }
+ if (ssl_data_alloc(&md5_out, MAX(out->data_len,16)) < 0) {
+ ssl_debug_printf("tls_prf: can't allocate md5 out\n");
goto free_sha;
- if (ssl_data_alloc(&seed, usage_len+rnd1->data_len+rnd2->data_len) < 0)
+ }
+ if (ssl_data_alloc(&seed, usage_len+rnd1->data_len+rnd2->data_len) < 0) {
+ ssl_debug_printf("tls_prf: can't allocate rnd %d\n",
+ usage_len+rnd1->data_len+rnd2->data_len);
goto free_md5;
+ }
ptr=seed.data;
memcpy(ptr,usage,usage_len); ptr+=usage_len;
@@ -1053,10 +1063,14 @@
/* initalize buffer for client/server seeds*/
s_l=secret->data_len/2 + secret->data_len%2;
- if (ssl_data_alloc(&s1, s_l) < 0)
+ if (ssl_data_alloc(&s1, s_l) < 0) {
+ ssl_debug_printf("tls_prf: can't allocate secret %d\n", s_l);
goto free_seed;
- if (ssl_data_alloc(&s2, s_l) < 0)
+ }
+ if (ssl_data_alloc(&s2, s_l) < 0) {
+ ssl_debug_printf("tls_prf: can't allocate secret(2) %d\n", s_l);
goto free_s1;
+ }
memcpy(s1.data,secret->data,s_l);
memcpy(s2.data,secret->data + (secret->data_len - s_l),s_l);
@@ -1271,6 +1285,16 @@
gint needed;
guint8 *ptr,*c_wk,*s_wk,*c_mk,*s_mk,*c_iv = _iv_c,*s_iv = _iv_s;
+ /* check for enough info to proced */
+ guint need_all = SSL_CIPHER|SSL_CLIENT_RANDOM|SSL_SERVER_RANDOM|SSL_VERSION;
+ guint need_any = SSL_MASTER_SECRET | SSL_PRE_MASTER_SECRET;
+ if (((ssl_session->state & need_all) != need_all) || ((ssl_session->state & need_any) == 0)) {
+ ssl_debug_printf("ssl_generate_keyring_material not enough data to generate key "
+ "(0x%02X required 0x%02X or 0x%02X)\n", ssl_session->state,
+ need_all|SSL_MASTER_SECRET, need_all|SSL_PRE_MASTER_SECRET);
+ return -1;
+ }
+
/* if master_key is not yet generate, create it now*/
if (!(ssl_session->state & SSL_MASTER_SECRET)) {
ssl_debug_printf("ssl_generate_keyring_material:PRF(pre_master_secret)\n");
@@ -1281,6 +1305,10 @@
return -1;
}
ssl_print_string("master secret",&ssl_session->master_secret);
+
+ /* the pre-master secret has been 'consumend' so we must clear it now */
+ ssl_session->state &= ~SSL_PRE_MASTER_SECRET;
+ ssl_session->state |= SSL_MASTER_SECRET;
}
/* Compute the key block. First figure out how much data we need*/
@@ -1292,7 +1320,7 @@
key_block.data_len = needed;
key_block.data = g_malloc(needed);
if (!key_block.data) {
- ssl_debug_printf("ssl_generate_keyring_material can't allacate key_block\n");
+ ssl_debug_printf("ssl_generate_keyring_material can't allocate key_block (len %d)\n", needed);
return -1;
}
ssl_debug_printf("ssl_generate_keyring_material sess key generation\n");
@@ -1467,6 +1495,7 @@
ssl_debug_printf("ssl_generate_keyring_material: client seq %d, server seq %d\n",
ssl_session->client_new->seq, ssl_session->server_new->seq);
g_free(key_block.data);
+ ssl->state |= SSL_HAVE_SESSION_KEY;
return 0;
fail:
@@ -1520,6 +1549,7 @@
This force keying material regeneration in
case we're renegotiating */
ssl_session->state &= ~(SSL_MASTER_SECRET|SSL_HAVE_SESSION_KEY);
+ ssl_session->state |= SSL_PRE_MASTER_SECRET;
return 0;
}
@@ -2263,9 +2293,9 @@
ssl_session->client_random.data = ssl_session->_client_random;
ssl_session->server_random.data = ssl_session->_server_random;
ssl_session->master_secret.data_len = 48;
- ssl_session->server_data_for_iv.data = 0;
+ ssl_session->server_data_for_iv.data_len = 0;
ssl_session->server_data_for_iv.data = ssl_session->_server_data_for_iv;
- ssl_session->client_data_for_iv.data = 0;
+ ssl_session->client_data_for_iv.data_len = 0;
ssl_session->client_data_for_iv.data = ssl_session->_client_data_for_iv;
ssl_session->app_data_segment.data=NULL;
ssl_session->app_data_segment.data_len=0;
Index: epan/dissectors/packet-ssl-utils.h
===================================================================
--- epan/dissectors/packet-ssl-utils.h (revision 25665)
+++ epan/dissectors/packet-ssl-utils.h (working copy)
@@ -195,6 +195,7 @@
#define SSL_HAVE_SESSION_KEY 8
#define SSL_VERSION 0x10
#define SSL_MASTER_SECRET 0x20
+#define SSL_PRE_MASTER_SECRET 0x40
#define SSL_CIPHER_MODE_STREAM 0
#define SSL_CIPHER_MODE_CBC 1
@@ -287,6 +288,7 @@
StringInfo server_random;
StringInfo client_random;
StringInfo master_secret;
+ /* the data store for this StringInfo must be allocated explicitly with a capture lifetime scope */
StringInfo pre_master_secret;
guchar _server_data_for_iv[24];
StringInfo server_data_for_iv;
Index: epan/dissectors/packet-ssl.c
===================================================================
--- epan/dissectors/packet-ssl.c (revision 25665)
+++ epan/dissectors/packet-ssl.c (working copy)
@@ -1872,16 +1872,6 @@
if (!ssl)
break;
- /* check for required session data */
- ssl_debug_printf("dissect_ssl3_handshake found SSL_HND_CLIENT_KEY_EXCHG state 0x%X\n",
- ssl->state);
- if ((ssl->state & (SSL_CIPHER|SSL_CLIENT_RANDOM|SSL_SERVER_RANDOM|SSL_VERSION)) !=
- (SSL_CIPHER|SSL_CLIENT_RANDOM|SSL_SERVER_RANDOM|SSL_VERSION)) {
- ssl_debug_printf("dissect_ssl3_handshake not enough data to generate key (required 0x%02X)\n",
- (SSL_CIPHER|SSL_CLIENT_RANDOM|SSL_SERVER_RANDOM|SSL_VERSION));
- break;
- }
-
/* get encrypted data, on tls1 we have to skip two bytes
* (it's the encrypted len and should be equal to record len - 2)
*/
@@ -1916,7 +1906,7 @@
ssl_debug_printf("dissect_ssl3_handshake can't generate keyring material\n");
break;
}
- ssl->state |= SSL_HAVE_SESSION_KEY;
+
ssl_save_session(ssl, ssl_session_hash);
ssl_debug_printf("dissect_ssl3_handshake session keys succesfully generated\n");
}
@@ -2246,20 +2236,11 @@
/* if we have restored a session now we can have enought material
* to build session key, check it out*/
- if ((ssl->state &
- (SSL_CIPHER|SSL_CLIENT_RANDOM|SSL_SERVER_RANDOM|SSL_VERSION|SSL_MASTER_SECRET)) !=
- (SSL_CIPHER|SSL_CLIENT_RANDOM|SSL_SERVER_RANDOM|SSL_VERSION|SSL_MASTER_SECRET)) {
- ssl_debug_printf("dissect_ssl3_hnd_srv_hello not enough data to generate key (required 0x%02X)\n",
- (SSL_CIPHER|SSL_CLIENT_RANDOM|SSL_SERVER_RANDOM|SSL_VERSION|SSL_MASTER_SECRET));
- goto no_cipher;
- }
-
ssl_debug_printf("dissect_ssl3_hnd_srv_hello trying to generate keys\n");
if (ssl_generate_keyring_material(ssl)<0) {
ssl_debug_printf("dissect_ssl3_hnd_srv_hello can't generate keyring material\n");
goto no_cipher;
}
- ssl->state |= SSL_HAVE_SESSION_KEY;
}
no_cipher:
@@ -2908,7 +2889,7 @@
{
tvb_memcpy(tvb,ssl->session_id.data, offset, session_id_length);
ssl->session_id.data_len = session_id_length;
- ssl->state &= ~(SSL_HAVE_SESSION_KEY|SSL_MASTER_SECRET|
+ ssl->state &= ~(SSL_HAVE_SESSION_KEY|SSL_MASTER_SECRET|SSL_PRE_MASTER_SECRET
SSL_CIPHER|SSL_SERVER_RANDOM);
}
offset += session_id_length;
@@ -3575,20 +3556,11 @@
ssl_debug_printf("ssl_set_master_secret set MASTER SECRET -> state 0x%02X\n", ssl->state);
}
- if ((ssl->state &
- (SSL_CIPHER|SSL_CLIENT_RANDOM|SSL_SERVER_RANDOM|SSL_VERSION|SSL_MASTER_SECRET)) !=
- (SSL_CIPHER|SSL_CLIENT_RANDOM|SSL_SERVER_RANDOM|SSL_VERSION|SSL_MASTER_SECRET)) {
- ssl_debug_printf("ssl_set_master_secret not enough data to generate key (has 0x%02X but required 0x%02X)\n",
- ssl->state, (SSL_CIPHER|SSL_CLIENT_RANDOM|SSL_SERVER_RANDOM|SSL_VERSION|SSL_MASTER_SECRET));
- return;
- }
-
ssl_debug_printf("ssl_set_master_secret trying to generate keys\n");
if (ssl_generate_keyring_material(ssl)<0) {
ssl_debug_printf("ssl_set_master_secret can't generate keyring material\n");
return;
}
- ssl->state |= SSL_HAVE_SESSION_KEY;
/* chenge ciphers immediately */
ssl_change_cipher(ssl, TRUE);
- Follow-Ups:
- Re: [Wireshark-dev] [patch] sparse fix to SSL decryption code
- From: Jaap Keuter
- Re: [Wireshark-dev] [patch] sparse fix to SSL decryption code
- Prev by Date: Re: [Wireshark-dev] Query on Field Registration
- Next by Date: [Wireshark-dev] same call
- Previous by thread: Re: [Wireshark-dev] Header file of FIX Protocol
- Next by thread: Re: [Wireshark-dev] [patch] sparse fix to SSL decryption code
- Index(es):