[ssl] dissect handshake messages even if we have no tree
authorMartin Kaiser <wireshark@kaiser.cx>
Tue, 20 Oct 2015 16:41:46 +0000 (18:41 +0200)
committerMichael Mann <mmann78@netscape.net>
Tue, 27 Oct 2015 21:47:57 +0000 (21:47 +0000)
this is to make sure that all expert info we see in the
main window will also appear in the expert info window

the sample capture from bug 11561 shows this problem:
without this patch, the expert info with severity 'error'
don't show up in the expert info window

Change-Id: Ia71ae7e248f57bf1344cf722ac57e74c517828d5
Reviewed-on: https://code.wireshark.org/review/11246
Petri-Dish: Michael Mann <mmann78@netscape.net>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
epan/dissectors/packet-dtls.c
epan/dissectors/packet-ssl-utils.c
epan/dissectors/packet-ssl.c

index c07082db6196a1bc30d5475c22c4f645bba8ac7c..512c47d777e0e3e96b786f55379599b23364a2c7 100644 (file)
@@ -1104,6 +1104,7 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo,
   guint32      fragment_length;
   gboolean     first_iteration;
   guint32      reassembled_length;
+  tvbuff_t     *sub_tvb;
 
   msg_type_str    = NULL;
   first_iteration = TRUE;
@@ -1302,35 +1303,31 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo,
             }
         }
 
-      if (ssl_hand_tree || ssl)
+        if (fragmented && !new_tvb)
         {
-          tvbuff_t *sub_tvb = NULL;
-
-          if (fragmented && !new_tvb)
-            {
-              /* Skip fragmented messages not reassembled yet */
-              continue;
-            }
+          /* Skip fragmented messages not reassembled yet */
+          continue;
+        }
 
-          if (new_tvb)
-            {
-              sub_tvb = new_tvb;
-            }
-          else
-            {
-              sub_tvb = tvb_new_subset_length(tvb, offset, fragment_length);
-            }
+        if (new_tvb)
+        {
+          sub_tvb = new_tvb;
+        }
+        else
+        {
+          sub_tvb = tvb_new_subset_length(tvb, offset, fragment_length);
+        }
 
-          /* now dissect the handshake message, if necessary */
-          switch ((HandshakeType) msg_type) {
+        /* now dissect the handshake message, if necessary */
+        switch ((HandshakeType) msg_type) {
           case SSL_HND_HELLO_REQUEST:
             /* hello_request has no fields, so nothing to do! */
             break;
 
           case SSL_HND_CLIENT_HELLO:
             if (ssl) {
-                /* ClientHello is first packet so set direction */
-                ssl_set_server(session, &pinfo->dst, pinfo->ptype, pinfo->destport);
+              /* ClientHello is first packet so set direction */
+              ssl_set_server(session, &pinfo->dst, pinfo->ptype, pinfo->destport);
             }
             ssl_dissect_hnd_cli_hello(&dissect_dtls_hf, sub_tvb, pinfo,
                                       ssl_hand_tree, 0, length, session, ssl,
@@ -1401,8 +1398,6 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo,
           case SSL_HND_ENCRYPTED_EXTS:
             /* TODO: does this need further dissection? */
             break;
-          }
-
         }
     }
 }
index 14fd94b2ec521a6c6df3fb0bacd3560eca8a8f51..487b31f30443edc0d8a7b830098ce414bf7cf929 100644 (file)
@@ -5312,61 +5312,59 @@ ssl_dissect_hnd_hello_common(ssl_common_dissect_t *hf, tvbuff_t *tvb,
     guint8       sessid_length;
     proto_tree  *rnd_tree;
 
-    if (tree || ssl) {
-        if (ssl) {
-            StringInfo *rnd;
-            if (from_server)
-                rnd = &ssl->server_random;
-            else
-                rnd = &ssl->client_random;
+    if (ssl) {
+        StringInfo *rnd;
+        if (from_server)
+            rnd = &ssl->server_random;
+        else
+            rnd = &ssl->client_random;
 
-            /* save provided random for later keyring generation */
-            tvb_memcpy(tvb, rnd->data, offset, 32);
-            rnd->data_len = 32;
-            if (from_server)
-                ssl->state |= SSL_SERVER_RANDOM;
-            else
-                ssl->state |= SSL_CLIENT_RANDOM;
-            ssl_debug_printf("%s found %s RANDOM -> state 0x%02X\n", G_STRFUNC,
-                             from_server ? "SERVER" : "CLIENT", ssl->state);
-        }
+        /* save provided random for later keyring generation */
+        tvb_memcpy(tvb, rnd->data, offset, 32);
+        rnd->data_len = 32;
+        if (from_server)
+            ssl->state |= SSL_SERVER_RANDOM;
+        else
+            ssl->state |= SSL_CLIENT_RANDOM;
+        ssl_debug_printf("%s found %s RANDOM -> state 0x%02X\n", G_STRFUNC,
+                from_server ? "SERVER" : "CLIENT", ssl->state);
+    }
 
-        rnd_tree = proto_tree_add_subtree(tree, tvb, offset, 32,
-                                          hf->ett.hs_random, NULL, "Random");
+    rnd_tree = proto_tree_add_subtree(tree, tvb, offset, 32,
+            hf->ett.hs_random, NULL, "Random");
 
-        /* show the time */
-        gmt_unix_time.secs  = tvb_get_ntohl(tvb, offset);
-        gmt_unix_time.nsecs = 0;
-        proto_tree_add_time(rnd_tree, hf->hf.hs_random_time,
-                            tvb, offset, 4, &gmt_unix_time);
-        offset += 4;
+    /* show the time */
+    gmt_unix_time.secs  = tvb_get_ntohl(tvb, offset);
+    gmt_unix_time.nsecs = 0;
+    proto_tree_add_time(rnd_tree, hf->hf.hs_random_time,
+            tvb, offset, 4, &gmt_unix_time);
+    offset += 4;
 
-        /* show the random bytes */
-        proto_tree_add_item(rnd_tree, hf->hf.hs_random_bytes,
-                            tvb, offset, 28, ENC_NA);
-        offset += 28;
+    /* show the random bytes */
+    proto_tree_add_item(rnd_tree, hf->hf.hs_random_bytes,
+            tvb, offset, 28, ENC_NA);
+    offset += 28;
 
-        /* show the session id (length followed by actual Session ID) */
-        sessid_length = tvb_get_guint8(tvb, offset);
-        proto_tree_add_item(tree, hf->hf.hs_session_id_len,
-                            tvb, offset, 1, ENC_BIG_ENDIAN);
-        offset++;
+    /* show the session id (length followed by actual Session ID) */
+    sessid_length = tvb_get_guint8(tvb, offset);
+    proto_tree_add_item(tree, hf->hf.hs_session_id_len,
+            tvb, offset, 1, ENC_BIG_ENDIAN);
+    offset++;
 
-        if (ssl) {
-            /* save the authorative SID for later use in ChangeCipherSpec.
-             * (D)TLS restricts the SID to 32 chars, it does not make sense to
-             * save more, so ignore larger ones. */
-            if (from_server && sessid_length <= 32) {
-                tvb_memcpy(tvb, ssl->session_id.data, offset, sessid_length);
-                ssl->session_id.data_len = sessid_length;
-            }
-        }
-        if (sessid_length > 0) {
-            proto_tree_add_item(tree, hf->hf.hs_session_id,
-                                tvb, offset, sessid_length, ENC_NA);
-            offset += sessid_length;
+    if (ssl) {
+        /* save the authorative SID for later use in ChangeCipherSpec.
+         * (D)TLS restricts the SID to 32 chars, it does not make sense to
+         * save more, so ignore larger ones. */
+        if (from_server && sessid_length <= 32) {
+            tvb_memcpy(tvb, ssl->session_id.data, offset, sessid_length);
+            ssl->session_id.data_len = sessid_length;
         }
     }
+    if (sessid_length > 0) {
+        proto_tree_add_item(tree, hf->hf.hs_session_id,
+                tvb, offset, sessid_length, ENC_NA);
+        offset += sessid_length;
+    }
 
     return offset;
 }
index 1a7645e95344fb733317e622be371aea04f6d33d..e98c3f30911e2de6957cfcfc71745166235cba7c 100644 (file)
@@ -1984,19 +1984,16 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
         if (!msg_type_str)
             return;
 
-        /* PAOLO: if we are doing ssl decryption we must dissect some requests type */
-        if (ssl_hand_tree || ssl)
-        {
-            /* add nodes for the message type and message length */
-            proto_tree_add_uint(ssl_hand_tree, hf_ssl_handshake_type,
-                    tvb, offset, 1, msg_type);
-            offset += 1;
-            proto_tree_add_uint(ssl_hand_tree, hf_ssl_handshake_length,
-                    tvb, offset, 3, length);
-            offset += 3;
+        /* add nodes for the message type and message length */
+        proto_tree_add_uint(ssl_hand_tree, hf_ssl_handshake_type,
+                tvb, offset, 1, msg_type);
+        offset += 1;
+        proto_tree_add_uint(ssl_hand_tree, hf_ssl_handshake_length,
+                tvb, offset, 3, length);
+        offset += 3;
 
-            /* now dissect the handshake message, if necessary */
-            switch ((HandshakeType) msg_type) {
+        /* now dissect the handshake message, if necessary */
+        switch ((HandshakeType) msg_type) {
             case SSL_HND_HELLO_REQUEST:
                 /* hello_request has no fields, so nothing to do! */
                 break;
@@ -2007,13 +2004,13 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
                     ssl_set_server(session, &pinfo->dst, pinfo->ptype, pinfo->destport);
                 }
                 ssl_dissect_hnd_cli_hello(&dissect_ssl3_hf, tvb, pinfo,
-                                          ssl_hand_tree, offset, length, session, ssl,
-                                          NULL);
+                        ssl_hand_tree, offset, length, session, ssl,
+                        NULL);
                 break;
 
             case SSL_HND_SERVER_HELLO:
                 ssl_dissect_hnd_srv_hello(&dissect_ssl3_hf, tvb, pinfo, ssl_hand_tree,
-                                          offset, length, session, ssl);
+                        offset, length, session, ssl);
                 break;
 
             case SSL_HND_HELLO_VERIFY_REQUEST:
@@ -2024,8 +2021,8 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
                 /* no need to load keylog file here as it only links a previous
                  * master key with this Session Ticket */
                 ssl_dissect_hnd_new_ses_ticket(&dissect_ssl3_hf, tvb,
-                                               ssl_hand_tree, offset, ssl,
-                                               ssl_master_key_map.session);
+                        ssl_hand_tree, offset, ssl,
+                        ssl_master_key_map.session);
                 break;
 
             case SSL_HND_CERTIFICATE:
@@ -2056,18 +2053,18 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
                     break;
 
                 ssl_load_keyfile(ssl_options.keylog_filename, &ssl_keylog_file,
-                                 &ssl_master_key_map);
+                        &ssl_master_key_map);
                 /* try to find master key from pre-master key */
                 if (!ssl_generate_pre_master_secret(ssl, length, tvb, offset,
-                                                    ssl_options.psk,
-                                                    &ssl_master_key_map)) {
+                            ssl_options.psk,
+                            &ssl_master_key_map)) {
                     ssl_debug_printf("dissect_ssl3_handshake can't generate pre master secret\n");
                 }
                 break;
 
             case SSL_HND_FINISHED:
                 ssl_dissect_hnd_finished(&dissect_ssl3_hf, tvb, ssl_hand_tree,
-                                         offset, session, &ssl_hfs);
+                        offset, session, &ssl_hfs);
                 break;
 
             case SSL_HND_CERT_URL:
@@ -2085,11 +2082,7 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
             case SSL_HND_ENCRYPTED_EXTS:
                 dissect_ssl3_hnd_encrypted_exts(tvb, ssl_hand_tree, offset);
                 break;
-            }
-
         }
-        else
-            offset += 4;        /* skip the handshake header when handshake is not processed*/
 
         offset += length;
         first_iteration = FALSE; /* set up for next pass, if any */