Wireshark · Ethereal-dev: [Ethereal-dev] Patch for packet-ssl.c information display.

[Martin Warnes <martin@xxxxxxxxxxxxxxxxx>]

Please find attached a fix to the SSL dissector code for consideration
of inclusion.

Currently when dissecting a SSL packet with multiple messages the
Information column reflects the contents by displaying each message type
separated by column. i.e; "Certificate, Server Hello Done"

However if the SSL dissector is called as a sub-dissector of a higher
protocol dissector, and the TCP segment contains multiple higher level
protocol packets only the last SSL message content is displayed in the
Information column. This is because the SSL dissector issues a col_clear
when called and in this scenario each subsequent call to the SSL
dissector would clear the previous written message type.

This patch addresses the situation by maintaining a reference to the
last "packet seen" in the conversation data structure and only issues a
col_clear for the first call to the SSL dissector for a packet,
subsequent calls for the same packet will instead append a ", " to
maintain consistency with how the SSL dissector currently works.

I've attached a couple of pre/post patch screen shots from a dissector
for the Connect:Direct transfer protocol I'm currently developing which
hopefully illustrates this. Each Packet contains 2x Connect:Direct
protocol packets each with a TLS sub payload, prior to the patch only
the last TLS handshake message was displayed.

I had to modify the conversation code within the SSL dissector to
accommodate this patch and although I've tested against my corpus of
SSL/TLS captures as well as Fuzz testing it should probably be reviewed
by someone more familiar with the SSL dissector to ensure I've not made
a glaringly obvious mistake.

Regards .. Martin

Index: packet-ssl.c
===================================================================
--- packet-ssl.c	(revision 17656)
+++ packet-ssl.c	(working copy)
@@ -210,6 +210,13 @@
 static gint ett_pct_cert_suites		  = -1;
 static gint ett_pct_exch_suites		  = -1;
 
+struct ssl_conversation_data
+{
+  SslDecryptSession* ssl_session;
+  guint last_packet_seen;
+};
+struct ssl_conversation_data *conversation_data;
+
 typedef struct {
     unsigned int ssl_port;
     unsigned int decrypted_port;
@@ -1048,7 +1055,7 @@
                                proto_tree *tree, guint32 offset,
                                guint *conv_version,
                                gboolean *need_desegmentation,
-                               SslDecryptSession *conv_data);
+                               SslDecryptSession *ssl_session);
 
 /* change cipher spec dissector */
 static void dissect_ssl3_change_cipher_spec(tvbuff_t *tvb,
@@ -1066,7 +1073,7 @@
                                    proto_tree *tree, guint32 offset,
                                    guint32 record_length,
                                    guint *conv_version,
-                                   SslDecryptSession *conv_data, guint8 content_type);
+                                   SslDecryptSession *ssl_session, guint8 content_type);
 
 
 static void dissect_ssl3_hnd_cli_hello(tvbuff_t *tvb,
@@ -1169,16 +1176,14 @@
 static void
 dissect_ssl(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
-
     conversation_t *conversation;
-    void *conv_data;
     proto_item *ti         = NULL;
     proto_tree *ssl_tree   = NULL;
     guint32 offset         = 0;
     gboolean first_record_in_frame = TRUE;
     gboolean need_desegmentation;
+    guint* conv_version;
     SslDecryptSession* ssl_session = NULL;
-    guint* conv_version;
 
     /* Track the version using conversations to reduce the
      * chance that a packet that simply *looks* like a v2 or
@@ -1200,19 +1205,22 @@
         conversation = conversation_new(pinfo->fd->num, &pinfo->src, &pinfo->dst, pinfo->ptype,
                                         pinfo->srcport, pinfo->destport, 0);
     }
-    conv_data = conversation_get_proto_data(conversation, proto_ssl);
+    conversation_data = conversation_get_proto_data(conversation, proto_ssl);
     
     /* PAOLO: manage ssl decryption data */
     /*get a valid ssl session pointer*/ 
-    if (conv_data != NULL)
-        ssl_session = conv_data;
+    if (conversation_data != NULL)
+        ssl_session = conversation_data->ssl_session;
     else {
         SslService dummy;
-
+	conversation_data =
+	  se_alloc (sizeof (struct ssl_conversation_data));
+	
         ssl_session = se_alloc0(sizeof(SslDecryptSession));
         ssl_session_init(ssl_session);
         ssl_session->version = SSL_VER_UNKNOWN;
-        conversation_add_proto_data(conversation, proto_ssl, ssl_session);
+	conversation_data->ssl_session=ssl_session;
+        conversation_add_proto_data(conversation, proto_ssl, conversation_data);
             
         /* we need to know witch side of conversation is speaking*/
         if (ssl_packet_from_server(pinfo->srcport)) {
@@ -1252,9 +1260,16 @@
         col_set_str(pinfo->cinfo, COL_PROTOCOL, "SSL");
     }
 
-    /* clear the the info column */
-    if (check_col(pinfo->cinfo, COL_INFO))
+    /* clear the the info column for the first ssl packet in protocol packet */
+    if (pinfo->fd->num != conversation_data->last_packet_seen) {
+      if (check_col(pinfo->cinfo, COL_INFO))
         col_clear(pinfo->cinfo, COL_INFO);
+    }
+    else {
+      if (check_col(pinfo->cinfo, COL_INFO))
+	col_append_str(pinfo->cinfo, COL_INFO, ", ");
+    }
+    conversation_data->last_packet_seen=pinfo->fd->num;
 
     /* TCP packets and SSL records are orthogonal.
      * A tcp packet may contain multiple ssl records and an ssl