Wireshark · Wireshark-dev: [Wireshark-dev] Code sharing between TLS and DTLS

[Hauke Mehrtens <hauke@xxxxxxxxxx>]

The DTLS rfc6347 describes just the differences to tls (ssl) and these
two protocols are very similar. Currently the wireshark dissectors for
tls and dtls are not sharing much code, just at little in ssl-utils.c.

I did some improvements for the DTLS dissector and much of this was
copying the code from tls and doing some small modifications to it. I
would like to share more code between these two dissectors.

Attached is a patch which makes dtls use the ssl code for analyzing the
Server- and Client-Hello extensions.

I assume that it is possible to remove more than 50% or the code in
epan/dissectors/packet-dtls.c by just using the functions from
epan/dissectors/packet-ssl.c.

I am writing this to the mailing list to get some opinions if this is
the right approach for this problem or if it should stay like it is now.
If I get positive feedback I will try to make dtls use more code from
the ssl dissector.

Hauke

>From 2bcbb36633807bac966b492608fc9f2020866d8a Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@xxxxxxxxxx>
Date: Thu, 13 Jun 2013 17:12:44 +0200
Subject: [PATCH 2/2] dtls: use dissector for ServerHello extensions from ssl

DTLS is just a modification to TLS, the RFC only defines differences.
There are no big differences and it is possible to use the same
function to dissect the ServerHello message for both protocols. Use the
dissector from ssl also for the dtls handshake message.

This also adds support for better decoding some extensions.

Signed-off-by: Hauke Mehrtens <hauke@xxxxxxxxxx>
---
 epan/dissectors/packet-dtls.c |   99 ++---------------------------------------
 epan/dissectors/packet-ssl.c  |    2 +-
 epan/dissectors/packet-ssl.h  |    5 +++
 3 files changed, 9 insertions(+), 97 deletions(-)

diff --git a/epan/dissectors/packet-dtls.c b/epan/dissectors/packet-dtls.c
index 50ac3d7..d551767 100644
--- a/epan/dissectors/packet-dtls.c
+++ b/epan/dissectors/packet-dtls.c
@@ -66,6 +66,7 @@
 #include <epan/uat.h>
 #include <epan/sctpppids.h>
 #include <epan/exported_pdu.h>
+#include "packet-ssl.h"
 
 void proto_register_dtls(void);
 
@@ -118,10 +119,6 @@ static gint hf_dtls_handshake_session_id        = -1;
 static gint hf_dtls_handshake_comp_methods_len  = -1;
 static gint hf_dtls_handshake_comp_methods      = -1;
 static gint hf_dtls_handshake_comp_method       = -1;
-static gint hf_dtls_handshake_extensions_len    = -1;
-static gint hf_dtls_handshake_extension_type    = -1;
-static gint hf_dtls_handshake_extension_len     = -1;
-static gint hf_dtls_handshake_extension_data    = -1;
 static gint hf_dtls_handshake_session_ticket_lifetime_hint = -1;
 static gint hf_dtls_handshake_session_ticket_len = -1;
 static gint hf_dtls_handshake_session_ticket    = -1;
@@ -172,7 +169,6 @@ static gint hf_dtls_handshake_dnames            = -1;
 static gint hf_dtls_handshake_dname_len         = -1;
 static gint hf_dtls_handshake_dname             = -1;
 
-static gint hf_dtls_heartbeat_extension_mode          = -1;
 static gint hf_dtls_heartbeat_message                 = -1;
 static gint hf_dtls_heartbeat_message_type            = -1;
 static gint hf_dtls_heartbeat_message_payload_length  = -1;
@@ -198,7 +194,6 @@ static gint ett_dtls_handshake         = -1;
 static gint ett_dtls_heartbeat         = -1;
 static gint ett_dtls_cipher_suites     = -1;
 static gint ett_dtls_comp_methods      = -1;
-static gint ett_dtls_extension         = -1;
 static gint ett_dtls_random            = -1;
 static gint ett_dtls_new_ses_ticket    = -1;
 static gint ett_dtls_keyex_params      = -1;
@@ -1778,68 +1773,6 @@ dissect_dtls_hnd_hello_common(tvbuff_t *tvb, proto_tree *tree,
   return offset;
 }
 
-static gint
-dissect_dtls_hnd_hello_ext(tvbuff_t *tvb,
-                           proto_tree *tree, guint32 offset, guint32 left)
-{
-  guint16     extension_length;
-  guint16     ext_type;
-  guint16     ext_len;
-  proto_item *pi;
-  proto_tree *ext_tree;
-
-  if (left < 2)
-    return offset;
-
-  extension_length = tvb_get_ntohs(tvb, offset);
-  proto_tree_add_uint(tree, hf_dtls_handshake_extensions_len,
-                      tvb, offset, 2, extension_length);
-  offset += 2;
-  left   -= 2;
-
-  while (left >= 4)
-    {
-      ext_type = tvb_get_ntohs(tvb, offset);
-      ext_len  = tvb_get_ntohs(tvb, offset + 2);
-
-      pi = proto_tree_add_text(tree, tvb, offset, 4 + ext_len,
-                               "Extension: %s",
-                               val_to_str(ext_type,
-                                          tls_hello_extension_types,
-                                          "Unknown %u"));
-      ext_tree = proto_item_add_subtree(pi, ett_dtls_extension);
-      if (!ext_tree)
-        ext_tree = tree;
-
-      proto_tree_add_uint(ext_tree, hf_dtls_handshake_extension_type,
-                          tvb, offset, 2, ext_type);
-      offset += 2;
-
-      proto_tree_add_uint(ext_tree, hf_dtls_handshake_extension_len,
-                          tvb, offset, 2, ext_len);
-      offset += 2;
-
-      switch (ext_type) {
-      case SSL_HND_HELLO_EXT_HEARTBEAT:
-          proto_tree_add_item(ext_tree, hf_dtls_heartbeat_extension_mode,
-                              tvb, offset, 1, ENC_BIG_ENDIAN);
-          offset += ext_len;
-          break;
-      default:
-          proto_tree_add_bytes_format(ext_tree, hf_dtls_handshake_extension_data,
-                                      tvb, offset, ext_len, NULL,
-                                      "Data (%u byte%s)",
-                                      ext_len, plurality(ext_len, "", "s"));
-          offset += ext_len;
-          break;
-      }
-
-      left   -= 2 + 2 + ext_len;
-    }
-
-  return offset;
-}
-
 static void
 dissect_dtls_hnd_cli_hello(tvbuff_t *tvb,
                            proto_tree *tree, guint32 offset, guint32 length,
@@ -1970,7 +1903,7 @@ dissect_dtls_hnd_cli_hello(tvbuff_t *tvb,
 
       if (length > offset - start_offset)
         {
-          dissect_dtls_hnd_hello_ext(tvb, tree, offset,
+          dissect_ssl3_hnd_hello_ext(tvb, tree, offset,
                                               length -
                                               (offset - start_offset));
         }
@@ -2058,7 +1991,7 @@ dissect_dtls_hnd_srv_hello(tvbuff_t *tvb,
 
       if (length > offset - start_offset)
         {
-          offset = dissect_dtls_hnd_hello_ext(tvb, tree, offset,
+          offset = dissect_ssl3_hnd_hello_ext(tvb, tree, offset,
                                               length -
                                               (offset - start_offset));
         }
@@ -3125,26 +3058,6 @@ proto_register_dtls(void)
         FT_UINT8, BASE_DEC, VALS(ssl_31_compression_method), 0x0,
         NULL, HFILL }
     },
-    { &hf_dtls_handshake_extensions_len,
-      { "Extensions Length", "dtls.handshake.extensions_length",
-        FT_UINT16, BASE_DEC, NULL, 0x0,
-        "Length of hello extensions", HFILL }
-    },
-    { &hf_dtls_handshake_extension_type,
-      { "Type", "dtls.handshake.extension.type",
-        FT_UINT16, BASE_HEX, VALS(tls_hello_extension_types), 0x0,
-        "Hello extension type", HFILL }
-    },
-    { &hf_dtls_handshake_extension_len,
-      { "Length", "dtls.handshake.extension.len",
-        FT_UINT16, BASE_DEC, NULL, 0x0,
-        "Length of a hello extension", HFILL }
-    },
-    { &hf_dtls_handshake_extension_data,
-      { "Data", "dtls.handshake.extension.data",
-        FT_BYTES, BASE_NONE, NULL, 0x0,
-        "Hello Extension data", HFILL }
-    },
     { &hf_dtls_handshake_session_ticket_lifetime_hint,
       { "Session Ticket Lifetime Hint", "dtls.handshake.session_ticket_lifetime_hint",
         FT_UINT32, BASE_DEC, NULL, 0x0,
@@ -3392,11 +3305,6 @@ proto_register_dtls(void)
         FT_BYTES, BASE_NONE, NULL, 0x0,
         "Distinguished name of a CA that server trusts", HFILL }
     },
-    { &hf_dtls_heartbeat_extension_mode,
-      { "Mode", "dtls.handshake.extension.heartbeat.mode",
-        FT_UINT8, BASE_DEC, VALS(tls_heartbeat_mode), 0x0,
-        "Heartbeat extension mode", HFILL }
-    },
     { &hf_dtls_heartbeat_message,
       { "Heartbeat Message", "dtls.heartbeat_message",
         FT_NONE, BASE_NONE, NULL, 0x0,
@@ -3472,7 +3380,6 @@ proto_register_dtls(void)
     &ett_dtls_heartbeat,
     &ett_dtls_cipher_suites,
     &ett_dtls_comp_methods,
-    &ett_dtls_extension,
     &ett_dtls_random,
     &ett_dtls_new_ses_ticket,
     &ett_dtls_keyex_params,
diff --git a/epan/dissectors/packet-ssl.c b/epan/dissectors/packet-ssl.c
index 8f38d45..87852f9 100644
--- a/epan/dissectors/packet-ssl.c
+++ b/epan/dissectors/packet-ssl.c
@@ -2435,7 +2435,7 @@ dissect_ssl3_hnd_hello_common(tvbuff_t *tvb, proto_tree *tree,
     return session_id_length+33;
 }
 
-static gint
+gint
 dissect_ssl3_hnd_hello_ext(tvbuff_t *tvb,
                            proto_tree *tree, guint32 offset, guint32 left)
 {
diff --git a/epan/dissectors/packet-ssl.h b/epan/dissectors/packet-ssl.h
index 217ed77..98f70a6 100644
--- a/epan/dissectors/packet-ssl.h
+++ b/epan/dissectors/packet-ssl.h
@@ -26,6 +26,8 @@
 #define __PACKET_SSL_H__
 
 #include "ws_symbol_export.h"
+#include <epan/tvbuff.h>
+#include <epan/proto.h>
 
 WS_DLL_PUBLIC GHashTable *ssl_session_hash;
 
@@ -42,4 +44,7 @@ extern gboolean ssl_ignore_mac_failed;
 
 gchar* ssl_association_info(void);
 
+gint dissect_ssl3_hnd_hello_ext(tvbuff_t *tvb, proto_tree *tree,
+                                guint32 offset, guint32 left);
+
 #endif  /* __PACKET_SSL_H__ */
-- 
1.7.10.4