Wireshark · Wireshark-dev: [Wireshark-dev] [PATCH] packet-ssl.c: Modify use of col_clear

[Martin Warnes <martin@xxxxxxxxxxxxxxxxx>]

Hi,

This patch modifies the way the COL_INFO field is updated and when
col_info is used to clear the contents, see below for a more detailed
explanation.

The SSL dissector (packet-ssl) is capable of handling multiple handshake
message, if this occurs then the handshake protocol for each message is
appended to the column information field:

+ Transmission Control Protocol
+ Secure Socket Layer
    - TLSv1 Record Layer
       + Handshake Protocol: Server Hello
       + Handshake Protocol: Certificate
       + Handshake Protocol: Certificate Request
       + Handshake Protocol: Server Hello Done

The above results in an Info field of: "Server
Hello,Certificate,Certificate Request,Server Hello Done"

When the SSL payload is being carried within another protocol the Info
field is still maintained.

+ Transmission Control Protocol
+ MyProtocol Layer
    + Secure Socket Layer
        - TLSv1 Record Layer
            + Handshake Protocol: Server Hello
           + Handshake Protocol: Certificate
           + Handshake Protocol: Certificate Request
           + Handshake Protocol: Server Hello Done

Still results in an Info field of: "Server Hello,Certificate,Certificate
Request,Server Hello Done"

However I have run into case where there can be multiple higher level
protocol records within a single packet each carrying a SSL record

+ Transmission Control Protocol
+ MyProtocol Layer
    + Secure Socket Layer
        - TLSv1 Record Layer
            + Handshake Protocol: Server Hello
+ MyProtocol Layer
    + Secure Socket Layer
        - TLSv1 Record Layer
           + Handshake Protocol: Certificate
+ MyProtocol Layer
    + Secure Socket Layer
        - TLSv1 Record Layer
           + Handshake Protocol: Certificate Request
+ MyProtocol Layer
    + Secure Socket Layer
        - TLSv1 Record Layer
           + Handshake Protocol: Server Hello Done


This results in the Info column just displaying "Server Hello Done"
because each time the SSL dissector is called it it issues a "col_clear"
against COL_INFO.

My attached solution only issues the col_clear() for the first SSL
record in a packet  by maintaining a "last packet seen" reference in the
conversation. Previously the SSL dissector passed the ssl_session as the
conversation data, in order to accomodate my changes I added a
conversation structure to handle the ssl_session and the last packet
seen reference.

I've tested on a number os standard SSL captures and captures where SSL
is being carried as a sub payload and not seen any problems.

Thank you for considering this.

Regards .. Martin

Index: packet-ssl.c
===================================================================
--- packet-ssl.c	(revision 20004)
+++ packet-ssl.c	(working copy)
@@ -227,6 +227,13 @@
 static gchar* ssl_keys_list = NULL;
 static gchar* ssl_debug_file_name = NULL;
 
+struct ssl_conversation_data
+{
+  SslDecryptSession* ssl_session;
+  guint last_packet_seen;
+};
+struct ssl_conversation_data *conversation_data;
+
 /* Forward declaration we need below */
 void proto_reg_handoff_ssl(void);
 
@@ -568,19 +575,23 @@
         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;
     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(ssl_associations, pinfo->srcport, pinfo->ptype == PT_TCP)) {
@@ -616,13 +627,22 @@
      * figure out what flavor of SSL it is (assuming we don't
      * throw an exception before we get the chance to do so). */
     if (check_col(pinfo->cinfo, COL_PROTOCOL))
-    {
+						{
         col_set_str(pinfo->cinfo, COL_PROTOCOL, "SSL");
+						}
+
+    /* 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);
     }
-    /* clear the the info column */
-    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
      * record may be spread across multiple tcp packets.