ssl: really fix session resumption expert info

In a two-pass dissection with renegotiated sessions, the
is_session_resumed flag is not updated according to the current protocol
flow. Fix this by performing detection of abbreviated handshakes in
all cases, do not limit it to the decryption stage (where ssl != NULL).

Reset the resumption assumption after the first ChangeCipherSpec
(normally from the server side, but explicitly add this in case client
packets somehow arrive earlier in the capture). This should not have a
functional effect on normal TLS captures with Session Tickets.

Bug: 12793
Change-Id: I1eb2a8262b4e359b8c1d3d0a1e004a9e856bec8c
Reviewed-on: https://code.wireshark.org/review/17483
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>

diff --git a/epan/dissectors/packet-dtls.c b/epan/dissectors/packet-dtls.c
index 877027604226f75b4ceb827aa1eb0d0c6f93eae3..1f2bc3204f7f42dd86c3958e39b68d99b85e09b5 100644 (file)
--- a/epan/dissectors/packet-dtls.c
+++ b/epan/dissectors/packet-dtls.c
@@ -769,6 +769,11 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo,
         ssl_finalize_decryption(ssl, &dtls_master_key_map);
         ssl_change_cipher(ssl, ssl_packet_from_server(session, dtls_associations, pinfo));
     }
+    /* Heuristic: any later ChangeCipherSpec is not a resumption of this
+     * session. Set the flag after ssl_finalize_decryption such that it has
+     * a chance to use resume using Session Tickets. */
+    if (is_from_server)
+      session->is_session_resumed = FALSE;
     break;
   case SSL_ID_ALERT:
     {
@@ -1300,8 +1305,8 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo,
             break;
 
           case SSL_HND_SVR_HELLO_DONE:
-            if (ssl)
-              ssl->state |= SSL_SERVER_HELLO_DONE;
+            /* This is not an abbreviated handshake, it is certainly not resumed. */
+            session->is_session_resumed = FALSE;
             break;
 
           case SSL_HND_CERT_VERIFY:

diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c
index e48024a5527ec77f1b8cfa0caaf8f9471c5917d2..7c8f37622c62aa192f2b1e7caa2a07d41174e75c 100644 (file)
--- a/epan/dissectors/packet-ssl-utils.c
+++ b/epan/dissectors/packet-ssl-utils.c
@@ -5087,7 +5087,7 @@ ssl_dissect_change_cipher_spec(ssl_common_dissect_t *hf, tvbuff_t *tvb,
      * 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)) {
+        if (session->is_session_resumed) {
             const char *resumed = NULL;
             if (ssl->session_ticket.data_len) {
                 resumed = "Session Ticket";
@@ -5096,15 +5096,12 @@ ssl_dissect_change_cipher_spec(ssl_common_dissect_t *hf, tvbuff_t *tvb,
             }
             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);
-                session->is_session_resumed = FALSE;
             }
         } else {
             ssl_debug_printf("%s Not using Session resumption\n", G_STRFUNC);
-            session->is_session_resumed = FALSE;
         }
     }
     if (is_from_server && session->is_session_resumed)
@@ -5890,6 +5887,11 @@ ssl_dissect_hnd_srv_hello(ssl_common_dissect_t *hf, tvbuff_t *tvb,
     ssl_try_set_version(session, ssl, SSL_ID_HANDSHAKE, SSL_HND_SERVER_HELLO,
             is_dtls, tvb_get_ntohs(tvb, offset));
 
+    /* Initially assume that the session is resumed. If this is not the case, a
+     * ServerHelloDone will be observed before the ChangeCipherSpec message
+     * which will reset this flag. */
+    session->is_session_resumed = TRUE;
+
     /* show the server version */
     proto_tree_add_item(tree, hf->hf.hs_server_version, tvb,
                         offset, 2, ENC_BIG_ENDIAN);

diff --git a/epan/dissectors/packet-ssl-utils.h b/epan/dissectors/packet-ssl-utils.h
index 7dcc83203ad374f1dc3153ffe8ec3caf6b6123cc..25961ec12df305b74e1b02912173b2e553d00b3f 100644 (file)
--- a/epan/dissectors/packet-ssl-utils.h
+++ b/epan/dissectors/packet-ssl-utils.h
@@ -229,7 +229,6 @@ 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_NEW_SESSION_TICKET  (1<<10)
 
 #define SSL_EXTENDED_MASTER_SECRET_MASK (SSL_CLIENT_EXTENDED_MASTER_SECRET|SSL_SERVER_EXTENDED_MASTER_SECRET)

diff --git a/epan/dissectors/packet-ssl.c b/epan/dissectors/packet-ssl.c
index 67d4466d2684e5fce5cafddf0b3a730ca8a60125..d16d4f69989c2cb524b0ecd26f8e14af57a0ff15 100644 (file)
--- a/epan/dissectors/packet-ssl.c
+++ b/epan/dissectors/packet-ssl.c
@@ -1684,6 +1684,11 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
             ssl_finalize_decryption(ssl, &ssl_master_key_map);
             ssl_change_cipher(ssl, ssl_packet_from_server(session, ssl_associations, pinfo));
         }
+        /* Heuristic: any later ChangeCipherSpec is not a resumption of this
+         * session. Set the flag after ssl_finalize_decryption such that it has
+         * a chance to use resume using Session Tickets. */
+        if (is_from_server)
+          session->is_session_resumed = FALSE;
         break;
     case SSL_ID_ALERT:
     {
@@ -2063,8 +2068,8 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
                 break;
 
             case SSL_HND_SVR_HELLO_DONE:
-                if (ssl)
-                    ssl->state |= SSL_SERVER_HELLO_DONE;
+                /* This is not an abbreviated handshake, it is certainly not resumed. */
+                session->is_session_resumed = FALSE;
                 break;
 
             case SSL_HND_CERT_VERIFY: