TLS13: update NewSessionTicket dissection

The new ticket_age_add field resulted in a dissector exception. With
this fixed, the tls13-18-picotls-earlydata.pcap capture can now be fully
decrypted.

Also add validation for the ticket length (using ssl_add_vector).

Change-Id: I167038f682b47b2d1da020a8f241daaf7af22017
Ping-Bug: 12779
Reviewed-on: https://code.wireshark.org/review/19992
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>

4 files changed, 62 insertions, 21 deletions

diff --git a/epan/dissectors/packet-dtls.c b/epan/dissectors/packet-dtls.c
index d9c3a91923..ea4e0eea69 100644
--- a/epan/dissectors/packet-dtls.c
+++ b/epan/dissectors/packet-dtls.c
@@ -1279,8 +1279,8 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo,
           case SSL_HND_NEWSESSION_TICKET:
             /* 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_dtls_hf, sub_tvb,
-                                           ssl_hand_tree, 0, ssl,
+            ssl_dissect_hnd_new_ses_ticket(&dissect_dtls_hf, sub_tvb, pinfo,
+                                           ssl_hand_tree, 0, length, session, ssl,
                                            dtls_master_key_map.tickets);
             break;
 
diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c
index 1d1d75a0bf..e3cadb02cb 100644
--- a/epan/dissectors/packet-ssl-utils.c
+++ b/epan/dissectors/packet-ssl-utils.c
@@ -6890,18 +6890,30 @@ ssl_dissect_hnd_srv_hello(ssl_common_dissect_t *hf, tvbuff_t *tvb,
 
 /* New Session Ticket dissection. {{{ */
 void
-ssl_dissect_hnd_new_ses_ticket(ssl_common_dissect_t *hf, tvbuff_t *tvb,
-                               proto_tree *tree, guint32 offset,
-                               SslDecryptSession *ssl _U_,
+ssl_dissect_hnd_new_ses_ticket(ssl_common_dissect_t *hf, tvbuff_t *tvb, packet_info *pinfo,
+                               proto_tree *tree, guint32 offset, guint32 offset_end,
+                               const SslSession *session, SslDecryptSession *ssl _U_,
                                GHashTable *session_hash _U_)
 {
-    proto_tree  *subtree;
-    guint16      ticket_len;
+    /* https://tools.ietf.org/html/rfc5077#section-3.3 (TLS >= 1.0):
+     *  struct {
+     *      uint32 ticket_lifetime_hint;
+     *      opaque ticket<0..2^16-1>;
+     *  } NewSessionTicket;
+     *
+     * https://tools.ietf.org/html/draft-ietf-tls-tls13-18#section-4.5.1
+     *  struct {
+     *      uint32 ticket_lifetime;
+     *      uint32 ticket_age_add;
+     *      opaque ticket<1..2^16-1>;
+     *      Extension extensions<0..2^16-2>;
+     *  } NewSessionTicket;
+     */
+    proto_tree *subtree;
+    guint32     ticket_len;
+    gboolean    is_tls13 = session->version == TLSV1DOT3_VERSION;
 
-    /* length of session ticket, may be 0 if the server has sent the
-     * SessionTicket extension, but decides not to use one. */
-    ticket_len = tvb_get_ntohs(tvb, offset + 4);
-    subtree = proto_tree_add_subtree(tree, tvb, offset, 6 + ticket_len,
+    subtree = proto_tree_add_subtree(tree, tvb, offset, offset_end - offset,
                                      hf->ett.session_ticket, NULL,
                                      "TLS Session Ticket");
 
@@ -6910,16 +6922,26 @@ ssl_dissect_hnd_new_ses_ticket(ssl_common_dissect_t *hf, tvbuff_t *tvb,
                         tvb, offset, 4, ENC_BIG_ENDIAN);
     offset += 4;
 
-    /* opaque ticket (length, data) */
-    proto_tree_add_item(subtree, hf->hf.hs_session_ticket_len,
-                        tvb, offset, 2, ENC_BIG_ENDIAN);
+    if (is_tls13) {
+        /* for TLS 1.3: ticket_age_add */
+        proto_tree_add_item(subtree, hf->hf.hs_session_ticket_age_add,
+                            tvb, offset, 4, ENC_BIG_ENDIAN);
+        offset += 4;
+    }
+
+    /* opaque ticket<0..2^16-1> (with TLS 1.3 the minimum is 1) */
+    if (!ssl_add_vector(hf, tvb, pinfo, subtree, offset, offset_end, &ticket_len,
+                        hf->hf.hs_session_ticket_len, is_tls13 ? 1 : 0, G_MAXUINT16)) {
+        return;
+    }
     offset += 2;
+
     /* Content depends on implementation, so just show data! */
     proto_tree_add_item(subtree, hf->hf.hs_session_ticket,
                         tvb, offset, ticket_len, ENC_NA);
     /* save the session ticket to cache for ssl_finalize_decryption */
 #ifdef HAVE_LIBGCRYPT
-    if (ssl) {
+    if (ssl && !is_tls13) {
         tvb_ensure_bytes_exist(tvb, offset, ticket_len);
         ssl->session_ticket.data = (guchar*)wmem_realloc(wmem_file_scope(),
                                     ssl->session_ticket.data, ticket_len);
@@ -6935,6 +6957,18 @@ ssl_dissect_hnd_new_ses_ticket(ssl_common_dissect_t *hf, tvbuff_t *tvb,
         ssl->state |= SSL_NEW_SESSION_TICKET;
     }
 #endif
+    offset += ticket_len;
+
+    if (is_tls13) {
+        guint32     exts_len;
+
+        /* Extension extensions<0..2^16-2> */
+        if (!ssl_add_vector(hf, tvb, pinfo, subtree, offset, offset_end, &exts_len,
+                            hf->hf.hs_exts_len, 0, G_MAXUINT16)) {
+            return;
+        }
+        /* TODO handle ticket extensions (only early_data at the moment) */
+    }
 } /* }}} */
 
 void
diff --git a/epan/dissectors/packet-ssl-utils.h b/epan/dissectors/packet-ssl-utils.h
index 8ba3f1c451..f07890fe52 100644
--- a/epan/dissectors/packet-ssl-utils.h
+++ b/epan/dissectors/packet-ssl-utils.h
@@ -770,6 +770,7 @@ typedef struct ssl_common_dissect {
         gint hs_comp_methods;
         gint hs_comp_method;
         gint hs_session_ticket_lifetime_hint;
+        gint hs_session_ticket_age_add;
         gint hs_session_ticket_len;
         gint hs_session_ticket;
         gint hs_finished;
@@ -909,9 +910,9 @@ ssl_dissect_hnd_encrypted_extensions(ssl_common_dissect_t *hf, tvbuff_t *tvb, pa
                                      gboolean is_dtls);
 
 extern void
-ssl_dissect_hnd_new_ses_ticket(ssl_common_dissect_t *hf, tvbuff_t *tvb,
-                               proto_tree *tree, guint32 offset,
-                               SslDecryptSession *ssl,
+ssl_dissect_hnd_new_ses_ticket(ssl_common_dissect_t *hf, tvbuff_t *tvb, packet_info *pinfo,
+                               proto_tree *tree, guint32 offset, guint32 offset_end,
+                               const SslSession *session, SslDecryptSession *ssl,
                                GHashTable *session_hash);
 
 extern void
@@ -959,7 +960,7 @@ 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, -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, \
@@ -1524,6 +1525,12 @@ ssl_common_dissect_t name = {   \
         FT_UINT32, BASE_DEC, NULL, 0x0,                                 \
         "New Session Ticket Lifetime Hint", HFILL }                     \
     },                                                                  \
+    { & name .hf.hs_session_ticket_age_add,                             \
+      { "Session Ticket Age Add",                                       \
+        prefix ".handshake.session_ticket_age_add",                     \
+        FT_UINT32, BASE_DEC, NULL, 0x0,                                 \
+        "Random 32-bit value to obscure age of ticket", HFILL }         \
+    },                                                                  \
     { & name .hf.hs_session_ticket_len,                                 \
       { "Session Ticket Length", prefix ".handshake.session_ticket_length", \
         FT_UINT16, BASE_DEC, NULL, 0x0,                                 \
diff --git a/epan/dissectors/packet-ssl.c b/epan/dissectors/packet-ssl.c
index 7ae36978fb..a0d2789964 100644
--- a/epan/dissectors/packet-ssl.c
+++ b/epan/dissectors/packet-ssl.c
@@ -2112,8 +2112,8 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
             case SSL_HND_NEWSESSION_TICKET:
                 /* 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_dissect_hnd_new_ses_ticket(&dissect_ssl3_hf, tvb, pinfo,
+                        ssl_hand_tree, offset, offset + length, session, ssl,
                         ssl_master_key_map.tickets);
                 break;