ssl: add expert info for session resumption

Add expert info to the Change Cipher Spec tree when session resumption
is detected. This can be used as hint that decryption using a RSA key
file will not succeed because of missing key material.

The name of this expert info is "ssl.resumed" or "dtls.resumed" and the
expert info message is "This session reuses previously negotiated keys
(Session resumption)".

Change-Id: I4a83edb13417631c97d6cfc4a57e2086bd217878
Reviewed-on: https://code.wireshark.org/review/11583
Reviewed-by: Michael Mann <mmann78@netscape.net>
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
(cherry picked from commit e0cf8b6ccfdde4a256645979f39b2e46b92e963c)
Reviewed-on: https://code.wireshark.org/review/11866
Reviewed-by: Peter Wu <peter@lekensteyn.nl>

4 files changed, 81 insertions, 79 deletions

diff --git a/epan/dissectors/packet-dtls.c b/epan/dissectors/packet-dtls.c
index ce92f88ea5..4421acf812 100644
--- a/epan/dissectors/packet-dtls.c
+++ b/epan/dissectors/packet-dtls.c
@@ -86,7 +86,6 @@ static gint hf_dtls_record_epoch                = -1;
 static gint hf_dtls_record_sequence_number      = -1;
 static gint hf_dtls_record_length               = -1;
 static gint hf_dtls_record_appdata              = -1;
-static gint hf_dtls_change_cipher_spec          = -1;
 static gint hf_dtls_alert_message               = -1;
 static gint hf_dtls_alert_message_level         = -1;
 static gint hf_dtls_alert_message_description   = -1;
@@ -286,12 +285,6 @@ static gint dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo,
                                 SslSession *session, gint is_from_server,
                                 SslDecryptSession *conv_data);
 
-/* change cipher spec dissector */
-static void dissect_dtls_change_cipher_spec(tvbuff_t *tvb,
-                                            proto_tree *tree,
-                                            guint32 offset,
-                                            const SslSession *session, guint8 content_type);
-
 /* alert message dissector */
 static void dissect_dtls_alert(tvbuff_t *tvb, packet_info *pinfo,
                                proto_tree *tree, guint32 offset,
@@ -801,8 +794,9 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo,
   switch ((ContentType) content_type) {
   case SSL_ID_CHG_CIPHER_SPEC:
     col_append_str(pinfo->cinfo, COL_INFO, "Change Cipher Spec");
-    dissect_dtls_change_cipher_spec(tvb, dtls_record_tree,
-                                    offset, session, content_type);
+    ssl_dissect_change_cipher_spec(&dissect_dtls_hf, tvb, pinfo,
+                                   dtls_record_tree, offset, session,
+                                   is_from_server, ssl);
     if (ssl) {
         ssl_load_keyfile(dtls_options.keylog_filename, &dtls_keylog_file,
                          &dtls_master_key_map);
@@ -970,29 +964,6 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo,
   return offset;
 }
 
-/* dissects the change cipher spec protocol, filling in the tree */
-static void
-dissect_dtls_change_cipher_spec(tvbuff_t *tvb,
-                                proto_tree *tree, guint32 offset,
-                                const SslSession *session, guint8 content_type)
-{
-  /*
-   * struct {
-   *     enum { change_cipher_spec(1), (255) } type;
-   * } ChangeCipherSpec;
-   *
-   */
-  if (tree)
-    {
-      proto_item_set_text(tree,
-                          "%s Record Layer: %s Protocol: Change Cipher Spec",
-                          val_to_str_const(session->version, ssl_version_short_names, "SSL"),
-                          val_to_str_const(content_type, ssl_31_content_type, "unknown"));
-      proto_tree_add_item(tree, hf_dtls_change_cipher_spec, tvb,
-                          offset, 1, ENC_NA);
-    }
-}
-
 /* dissects the alert message, filling in the tree */
 static void
 dissect_dtls_alert(tvbuff_t *tvb, packet_info *pinfo,
@@ -1365,7 +1336,8 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo,
             break;
 
           case SSL_HND_SVR_HELLO_DONE:
-            /* server_hello_done has no fields, so nothing to do! */
+            if (ssl)
+              ssl->state |= SSL_SERVER_HELLO_DONE;
             break;
 
           case SSL_HND_CERT_VERIFY:
@@ -1699,11 +1671,6 @@ proto_register_dtls(void)
         FT_BYTES, BASE_NONE, NULL, 0x0,
         "Payload is encrypted application data", HFILL }
     },
-    { &hf_dtls_change_cipher_spec,
-      { "Change Cipher Spec Message", "dtls.change_cipher_spec",
-        FT_NONE, BASE_NONE, NULL, 0x0,
-        "Signals a change in cipher specifications", HFILL }
-    },
     { & hf_dtls_alert_message,
       { "Alert Message", "dtls.alert_message",
         FT_NONE, BASE_NONE, NULL, 0x0,
diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c
index 441a6504bc..4d0925da8d 100644
--- a/epan/dissectors/packet-ssl-utils.c
+++ b/epan/dissectors/packet-ssl-utils.c
@@ -4976,6 +4976,55 @@ ssldecrypt_uat_fld_password_chk_cb(void *r _U_, const char *p _U_, guint len _U_
 
 /** Begin of code related to dissection of wire data. */
 
+/* change_cipher_spec(20) dissection */
+void
+ssl_dissect_change_cipher_spec(ssl_common_dissect_t *hf, tvbuff_t *tvb,
+                               packet_info *pinfo, proto_tree *tree,
+                               guint32 offset, SslSession *session,
+                               gboolean is_from_server,
+                               const SslDecryptSession *ssl)
+{
+    /*
+     * struct {
+     *     enum { change_cipher_spec(1), (255) } type;
+     * } ChangeCipherSpec;
+     */
+    proto_item *ti;
+    proto_item_set_text(tree,
+            "%s Record Layer: %s Protocol: Change Cipher Spec",
+            val_to_str_const(session->version, ssl_version_short_names, "SSL"),
+            val_to_str_const(SSL_ID_CHG_CIPHER_SPEC, ssl_31_content_type, "unknown"));
+    ti = proto_tree_add_item(tree, hf->hf.change_cipher_spec, tvb, offset, 1, ENC_NA);
+
+    /* Use heuristics to detect an abbreviated handshake, assume that missing
+     * ServerHelloDone implies reusing previously negotiating keys. Then when
+     * a Session ID or ticket is present, it must be a resumed session.
+     * Normally this should be done at the Finished message, but that may be
+     * encrypted so we do it here, at the last cleartext message. */
+    if (is_from_server && ssl) {
+        if (!(ssl->state & SSL_SERVER_HELLO_DONE)) {
+            const char *resumed = NULL;
+            if (ssl->session_ticket.data_len) {
+                resumed = "Session Ticket";
+            } else if (ssl->session_id.data_len) {
+                resumed = "Session ID";
+            }
+            if (resumed) {
+                ssl_debug_printf("%s Session resumption using %s\n", G_STRFUNC, resumed);
+                session->is_session_resumed = TRUE;
+            } else {
+                /* Can happen if the capture somehow starts in the middle */
+                ssl_debug_printf("%s No Session resumption, missing packets?\n", G_STRFUNC);
+            }
+        } else {
+            ssl_debug_printf("%s Not using Session resumption\n", G_STRFUNC);
+        }
+    }
+    if (is_from_server && session->is_session_resumed)
+        expert_add_info(pinfo, ti, &hf->ei.resumed);
+}
+
+/** Begin of handshake(22) record dissections */
 /* dissect a list of hash algorithms, return the number of bytes dissected
    this is used for the signature algorithms extension and for the
    TLS1.2 certificate request. {{{ */
diff --git a/epan/dissectors/packet-ssl-utils.h b/epan/dissectors/packet-ssl-utils.h
index 4e3be07eea..f7a2496759 100644
--- a/epan/dissectors/packet-ssl-utils.h
+++ b/epan/dissectors/packet-ssl-utils.h
@@ -236,6 +236,7 @@ typedef struct _StringInfo {
 #define SSL_PRE_MASTER_SECRET   (1<<6)
 #define SSL_CLIENT_EXTENDED_MASTER_SECRET (1<<7)
 #define SSL_SERVER_EXTENDED_MASTER_SECRET (1<<8)
+#define SSL_SERVER_HELLO_DONE   (1<<9)
 
 #define SSL_EXTENDED_MASTER_SECRET_MASK (SSL_CLIENT_EXTENDED_MASTER_SECRET|SSL_SERVER_EXTENDED_MASTER_SECRET)
 
@@ -368,6 +369,7 @@ typedef struct _SslSession {
     /* The Application layer protocol if known (for STARTTLS support) */
     dissector_handle_t   app_handle;
     guint32              last_nontls_frame;
+    gboolean             is_session_resumed;
 } SslSession;
 
 /* RFC 5246, section 8.1 says that the master secret is always 48 bytes */
@@ -613,6 +615,7 @@ ssl_calculate_handshake_hash(SslDecryptSession *ssl_session, tvbuff_t *tvb, guin
 /* common header fields, subtrees and expert info for SSL and DTLS dissectors */
 typedef struct ssl_common_dissect {
     struct {
+        gint change_cipher_spec;
         gint hs_exts_len;
         gint hs_ext_alpn_len;
         gint hs_ext_alpn_list;
@@ -744,6 +747,7 @@ typedef struct ssl_common_dissect {
         expert_field hs_sig_hash_alg_len_bad;
         expert_field hs_cipher_suites_len_bad;
         expert_field hs_sig_hash_algs_bad;
+        expert_field resumed;
 
         /* do not forget to update SSL_COMMON_LIST_T and SSL_COMMON_EI_LIST! */
     } ei;
@@ -765,6 +769,13 @@ typedef struct {
     /* Do not forget to initialize ssl_hfs to -1 in packet-ssl.c! */
 } ssl_hfs_t;
 
+void
+ssl_dissect_change_cipher_spec(ssl_common_dissect_t *hf, tvbuff_t *tvb,
+                               packet_info *pinfo, proto_tree *tree,
+                               guint32 offset, SslSession *session,
+                               gboolean is_from_server,
+                               const SslDecryptSession *ssl);
+
 extern void
 ssl_dissect_hnd_cli_hello(ssl_common_dissect_t *hf, tvbuff_t *tvb,
                           packet_info *pinfo, proto_tree *tree, guint32 offset,
@@ -827,20 +838,25 @@ ssl_common_dissect_t name = {   \
         -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, \
         -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, \
         -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, \
-        -1, -1, -1                                                      \
+        -1, -1, -1, -1,                                                 \
     },                                                                  \
     /* ett */ {                                                         \
         -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, \
         -1, -1, -1, -1,                                                 \
     },                                                                  \
     /* ei */ {                                                          \
-        EI_INIT, EI_INIT, EI_INIT, EI_INIT,                             \
+        EI_INIT, EI_INIT, EI_INIT, EI_INIT, EI_INIT,                    \
     },                                                                  \
 }
 /* }}} */
 
 /* {{{ */
 #define SSL_COMMON_HF_LIST(name, prefix)                                \
+    { & name .hf.change_cipher_spec,                                    \
+      { "Change Cipher Spec Message", prefix ".change_cipher_spec",     \
+        FT_NONE, BASE_NONE, NULL, 0x0,                                  \
+        "Signals a change in cipher specifications", HFILL }            \
+    },                                                                  \
     { & name .hf.hs_exts_len,                                           \
       { "Extensions Length", prefix ".handshake.extensions_length",     \
         FT_UINT16, BASE_DEC, NULL, 0x0,                                 \
@@ -1380,6 +1396,10 @@ ssl_common_dissect_t name = {   \
     { & name .ei.hs_sig_hash_algs_bad, \
         { prefix ".handshake.sig_hash_algs.mult2", PI_MALFORMED, PI_ERROR, \
         "Hash Algorithm length must be a multiple of 2", EXPFILL } \
+    }, \
+    { & name .ei.resumed, \
+        { prefix ".resumed", PI_SEQUENCE, PI_NOTE, \
+        "This session reuses previously negotiated keys (Session resumption)", EXPFILL } \
     }
 /* }}} */
 
diff --git a/epan/dissectors/packet-ssl.c b/epan/dissectors/packet-ssl.c
index 04edcc8b6a..0da478a884 100644
--- a/epan/dissectors/packet-ssl.c
+++ b/epan/dissectors/packet-ssl.c
@@ -126,7 +126,6 @@ static gint hf_ssl2_record_is_escape          = -1;
 static gint hf_ssl2_record_padding_length     = -1;
 static gint hf_ssl2_msg_type                  = -1;
 static gint hf_pct_msg_type                   = -1;
-static gint hf_ssl_change_cipher_spec         = -1;
 static gint hf_ssl_alert_message              = -1;
 static gint hf_ssl_alert_message_level        = -1;
 static gint hf_ssl_alert_message_description  = -1;
@@ -492,12 +491,6 @@ static gint dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
                                 SslDecryptSession *conv_data,
                                 const gboolean first_record_in_frame);
 
-/* change cipher spec dissector */
-static void dissect_ssl3_change_cipher_spec(tvbuff_t *tvb,
-                                            proto_tree *tree,
-                                            guint32 offset,
-                                            const SslSession *session, const guint8 content_type);
-
 /* alert message dissector */
 static void dissect_ssl3_alert(tvbuff_t *tvb, packet_info *pinfo,
                                proto_tree *tree, guint32 offset,
@@ -1637,10 +1630,10 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
 
     switch ((ContentType) content_type) {
     case SSL_ID_CHG_CIPHER_SPEC:
-        ssl_debug_printf("dissect_ssl3_change_cipher_spec\n");
         col_append_str(pinfo->cinfo, COL_INFO, "Change Cipher Spec");
-        dissect_ssl3_change_cipher_spec(tvb, ssl_record_tree,
-                                        offset, session, content_type);
+        ssl_dissect_change_cipher_spec(&dissect_ssl3_hf, tvb, pinfo,
+                                       ssl_record_tree, offset, session,
+                                       is_from_server, ssl);
         if (ssl) {
             ssl_load_keyfile(ssl_options.keylog_filename, &ssl_keylog_file,
                              &ssl_master_key_map);
@@ -1770,29 +1763,6 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
     return offset;
 }
 
-/* dissects the change cipher spec procotol, filling in the tree */
-static void
-dissect_ssl3_change_cipher_spec(tvbuff_t *tvb,
-                                proto_tree *tree, guint32 offset,
-                                const SslSession *session, const guint8 content_type)
-{
-    /*
-     * struct {
-     *     enum { change_cipher_spec(1), (255) } type;
-     * } ChangeCipherSpec;
-     *
-     */
-    if (tree)
-    {
-        proto_item_set_text(tree,
-                            "%s Record Layer: %s Protocol: Change Cipher Spec",
-                            val_to_str_const(session->version, ssl_version_short_names, "SSL"),
-                            val_to_str_const(content_type, ssl_31_content_type, "unknown"));
-        proto_tree_add_item(tree, hf_ssl_change_cipher_spec, tvb,
-                            offset++, 1, ENC_NA);
-    }
-}
-
 /* dissects the alert message, filling in the tree */
 static void
 dissect_ssl3_alert(tvbuff_t *tvb, packet_info *pinfo,
@@ -2039,7 +2009,8 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
                 break;
 
             case SSL_HND_SVR_HELLO_DONE:
-                /* server_hello_done has no fields, so nothing to do! */
+                if (ssl)
+                    ssl->state |= SSL_SERVER_HELLO_DONE;
                 break;
 
             case SSL_HND_CERT_VERIFY:
@@ -3737,11 +3708,6 @@ proto_register_ssl(void)
             FT_UINT8, BASE_DEC, NULL, 0x0,
             "Length of padding at end of record", HFILL }
         },
-        { &hf_ssl_change_cipher_spec,
-          { "Change Cipher Spec Message", "ssl.change_cipher_spec",
-            FT_NONE, BASE_NONE, NULL, 0x0,
-            "Signals a change in cipher specifications", HFILL }
-        },
         { &hf_ssl_alert_message,
           { "Alert Message", "ssl.alert_message",
             FT_NONE, BASE_NONE, NULL, 0x0,