TLS13: dissect NewSessionTicket and CertificateRequest extensions

Process NewSessionTicket and CertificateRequest extensions for TLS 1.3.
CertificateExtension in draft -18 is replaced by Extension in the next
draft, so anticipate for that and start decoding this extensions vector.
Rename ssl_dissect_hnd_hello_ext to reflect extended functionality.

(Certificate Extensions SCT is still not dissected though, but at least
OCSP (status_request(_v2)) is supported now.)

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

4 files changed, 57 insertions, 55 deletions

diff --git a/epan/dissectors/packet-dtls.c b/epan/dissectors/packet-dtls.c
index d92f47fd9c..94e8472179 100644
--- a/epan/dissectors/packet-dtls.c
+++ b/epan/dissectors/packet-dtls.c
@@ -1276,7 +1276,7 @@ dissect_dtls_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_dtls_hf, sub_tvb, pinfo,
-                                           ssl_hand_tree, 0, length, session, ssl,
+                                           ssl_hand_tree, 0, length, session, ssl, TRUE,
                                            dtls_master_key_map.tickets);
             break;
 
@@ -1287,7 +1287,7 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo,
 
           case SSL_HND_CERTIFICATE:
             ssl_dissect_hnd_cert(&dissect_dtls_hf, sub_tvb, ssl_hand_tree, 0, length,
-                pinfo, session, ssl, dtls_key_hash, is_from_server);
+                pinfo, session, ssl, dtls_key_hash, is_from_server, TRUE);
             break;
 
           case SSL_HND_SERVER_KEY_EXCHG:
@@ -1295,7 +1295,7 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo,
             break;
 
           case SSL_HND_CERT_REQUEST:
-            ssl_dissect_hnd_cert_req(&dissect_dtls_hf, sub_tvb, pinfo, ssl_hand_tree, 0, length, session);
+            ssl_dissect_hnd_cert_req(&dissect_dtls_hf, sub_tvb, pinfo, ssl_hand_tree, 0, length, session, TRUE);
             break;
 
           case SSL_HND_SVR_HELLO_DONE:
diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c
index 23a348af0b..5e0fb09ad0 100644
--- a/epan/dissectors/packet-ssl-utils.c
+++ b/epan/dissectors/packet-ssl-utils.c
@@ -6714,7 +6714,7 @@ ssl_set_cipher(SslDecryptSession *ssl, guint16 cipher)
 
 /* Client Hello and Server Hello dissections. {{{ */
 static gint
-ssl_dissect_hnd_hello_ext(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tree,
+ssl_dissect_hnd_extension(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tree,
                           packet_info* pinfo, guint32 offset, guint32 offset_end, guint8 hnd_type,
                           SslSession *session, SslDecryptSession *ssl,
                           gboolean is_dtls);
@@ -6828,7 +6828,7 @@ ssl_dissect_hnd_cli_hello(ssl_common_dissect_t *hf, tvbuff_t *tvb,
 
     /* SSL v3.0 has no extensions, so length field can indeed be missing. */
     if (offset < offset_end) {
-        ssl_dissect_hnd_hello_ext(hf, tvb, tree, pinfo, offset,
+        ssl_dissect_hnd_extension(hf, tvb, tree, pinfo, offset,
                                   offset_end, SSL_HND_CLIENT_HELLO,
                                   session, ssl, dtls_hfs != NULL);
     }
@@ -6897,7 +6897,7 @@ ssl_dissect_hnd_srv_hello(ssl_common_dissect_t *hf, tvbuff_t *tvb,
 
     /* SSL v3.0 has no extensions, so length field can indeed be missing. */
     if (offset < offset_end) {
-        ssl_dissect_hnd_hello_ext(hf, tvb, tree, pinfo, offset,
+        ssl_dissect_hnd_extension(hf, tvb, tree, pinfo, offset,
                                   offset_end, SSL_HND_SERVER_HELLO,
                                   session, ssl, is_dtls);
     }
@@ -6908,8 +6908,8 @@ ssl_dissect_hnd_srv_hello(ssl_common_dissect_t *hf, tvbuff_t *tvb,
 void
 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)
+                               SslSession *session, SslDecryptSession *ssl,
+                               gboolean is_dtls, GHashTable *session_hash)
 {
     /* https://tools.ietf.org/html/rfc5077#section-3.3 (TLS >= 1.0):
      *  struct {
@@ -6974,14 +6974,9 @@ ssl_dissect_hnd_new_ses_ticket(ssl_common_dissect_t *hf, tvbuff_t *tvb, packet_i
     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) */
+        ssl_dissect_hnd_extension(hf, tvb, subtree, pinfo, offset,
+                                  offset_end, SSL_HND_NEWSESSION_TICKET,
+                                  session, ssl, is_dtls);
     }
 } /* }}} */
 
@@ -7001,24 +6996,24 @@ ssl_dissect_hnd_hello_retry_request(ssl_common_dissect_t *hf, tvbuff_t *tvb,
                         offset, 2, ENC_BIG_ENDIAN);
     offset += 2;
 
-    /* remaining data are extensions */
-    ssl_dissect_hnd_hello_ext(hf, tvb, tree, pinfo, offset,
+    ssl_dissect_hnd_extension(hf, tvb, tree, pinfo, offset,
                               offset_end, SSL_HND_HELLO_RETRY_REQUEST,
                               session, ssl, is_dtls);
 }
 
 void
 ssl_dissect_hnd_encrypted_extensions(ssl_common_dissect_t *hf, tvbuff_t *tvb,
-                                     packet_info* pinfo, proto_tree *tree, guint32 offset, guint32 length,
+                                     packet_info* pinfo, proto_tree *tree, guint32 offset, guint32 offset_end,
                                      SslSession *session, SslDecryptSession *ssl,
                                      gboolean is_dtls)
 {
-    /* struct {
+    /* https://tools.ietf.org/html/draft-ietf-tls-tls13-18#section-4.3.1
+     * struct {
      *     Extension extensions<0..2^16-1>;
      * } EncryptedExtensions;
      */
-    ssl_dissect_hnd_hello_ext(hf, tvb, tree, pinfo, offset,
-                              offset + length, SSL_HND_ENCRYPTED_EXTENSIONS,
+    ssl_dissect_hnd_extension(hf, tvb, tree, pinfo, offset,
+                              offset_end, SSL_HND_ENCRYPTED_EXTENSIONS,
                               session, ssl, is_dtls);
 }
 
@@ -7026,8 +7021,8 @@ ssl_dissect_hnd_encrypted_extensions(ssl_common_dissect_t *hf, tvbuff_t *tvb,
 void
 ssl_dissect_hnd_cert(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tree,
                      guint32 offset, guint32 offset_end, packet_info *pinfo,
-                     const SslSession *session, SslDecryptSession *ssl _U_,
-                     GHashTable *key_hash _U_, gint is_from_server)
+                     SslSession *session, SslDecryptSession *ssl _U_,
+                     GHashTable *key_hash _U_, gboolean is_from_server, gboolean is_dtls)
 {
     /* opaque ASN.1Cert<1..2^24-1>;
      *
@@ -7151,16 +7146,9 @@ ssl_dissect_hnd_cert(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tree,
 
                     /* TLS 1.3: Extension extensions<0..2^16-1> */
                     if (session->version == TLSV1DOT3_VERSION) {
-                        guint32 extensions_length;
-                        if (!ssl_add_vector(hf, tvb, pinfo, subtree, offset, next_offset, &extensions_length,
-                                            hf->hf.hs_exts_len, 0, G_MAXUINT16)) {
-                            return;
-                        }
-                        offset += 2;
-
-                        // XXX dissect OCSP and SCT extensions
-                        // https://tools.ietf.org/html/draft-ietf-tls-tls13-18#section-4.4.1.1
-                        offset += extensions_length;
+                        offset = ssl_dissect_hnd_extension(hf, tvb, subtree, pinfo, offset,
+                                                           next_offset, SSL_HND_CERTIFICATE,
+                                                           session, ssl, is_dtls);
                     }
                 }
             }
@@ -7177,7 +7165,7 @@ ssl_dissect_hnd_cert(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tree,
 void
 ssl_dissect_hnd_cert_req(ssl_common_dissect_t *hf, tvbuff_t *tvb, packet_info *pinfo,
                          proto_tree *tree, guint32 offset, guint32 offset_end,
-                         const SslSession *session)
+                         SslSession *session, gboolean is_dtls)
 {
     /* From SSL 3.0 and up (note that since TLS 1.1 certificate_authorities can be empty):
      *    enum {
@@ -7334,7 +7322,16 @@ ssl_dissect_hnd_cert_req(ssl_common_dissect_t *hf, tvbuff_t *tvb, packet_info *p
         }
     }
 
-    /* TODO Extensions specific to certificates (TLS 1.3). */
+    /* TODO this is not valid for TLS 1.3 draft -18. When draft -19 is released, check if this is still correct. */
+    if (session->version == TLSV1DOT3_VERSION) {
+        /*
+         * SslDecryptSession pointer is NULL because Certificate Extensions
+         * should not influence decryption state.
+         */
+        ssl_dissect_hnd_extension(hf, tvb, tree, pinfo, offset,
+                                  offset_end, SSL_HND_CERT_REQUEST,
+                                  session, NULL, is_dtls);
+    }
 }
 /* Certificate and Certificate Request dissections. }}} */
 
@@ -7452,26 +7449,26 @@ ssl_dissect_hnd_cert_url(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tr
     }
 } /* }}} */
 
-/* Client Hello and Server Hello TLS extensions dissection. {{{ */
+/* Dissection of TLS Extensions in Client Hello, Server Hello, etc. {{{ */
 static gint
-ssl_dissect_hnd_hello_ext(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tree,
+ssl_dissect_hnd_extension(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tree,
                           packet_info* pinfo, guint32 offset, guint32 offset_end, guint8 hnd_type,
                           SslSession *session, SslDecryptSession *ssl,
                           gboolean is_dtls)
 {
-    guint16     extension_length;
+    guint32     exts_len;
     guint16     ext_type;
     guint32     ext_len;
     guint32     next_offset;
     proto_tree *ext_tree;
 
-    if (offset_end - offset < 2)
-        return offset;
-
-    extension_length = tvb_get_ntohs(tvb, offset);
-    proto_tree_add_uint(tree, hf->hf.hs_exts_len,
-                        tvb, offset, 2, extension_length);
+    /* Extension extensions<0..2^16-2> (for TLS 1.3 HRR/CR min-length is 2) */
+    if (!ssl_add_vector(hf, tvb, pinfo, tree, offset, offset_end, &exts_len,
+                        hf->hf.hs_exts_len, 0, G_MAXUINT16)) {
+        return offset_end;
+    }
     offset += 2;
+    offset_end = offset + exts_len;
 
     while (offset_end - offset >= 4)
     {
@@ -7605,6 +7602,11 @@ ssl_dissect_hnd_hello_ext(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *t
         }
     }
 
+    /* Check if Extensions vector is correctly terminated. */
+    if (!ssl_end_vector(hf, tvb, pinfo, tree, offset, offset_end)) {
+        offset = offset_end;
+    }
+
     return offset;
 } /* }}} */
 
diff --git a/epan/dissectors/packet-ssl-utils.h b/epan/dissectors/packet-ssl-utils.h
index 5eb66ba084..690ccb0ca9 100644
--- a/epan/dissectors/packet-ssl-utils.h
+++ b/epan/dissectors/packet-ssl-utils.h
@@ -903,26 +903,26 @@ ssl_dissect_hnd_hello_retry_request(ssl_common_dissect_t *hf, tvbuff_t *tvb, pac
 
 extern void
 ssl_dissect_hnd_encrypted_extensions(ssl_common_dissect_t *hf, tvbuff_t *tvb, packet_info* pinfo,
-                                     proto_tree *tree, guint32 offset, guint32 length,
+                                     proto_tree *tree, guint32 offset, guint32 offset_end,
                                      SslSession *session, SslDecryptSession *ssl,
                                      gboolean is_dtls);
 
 extern void
 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);
+                               SslSession *session, SslDecryptSession *ssl,
+                               gboolean is_dtls, GHashTable *session_hash);
 
 extern void
 ssl_dissect_hnd_cert(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tree,
                      guint32 offset, guint32 offset_end, packet_info *pinfo,
-                     const SslSession *session, SslDecryptSession *ssl,
-                     GHashTable *key_hash, gint is_from_server);
+                     SslSession *session, SslDecryptSession *ssl,
+                     GHashTable *key_hash, gboolean is_from_server, gboolean is_dtls);
 
 extern void
 ssl_dissect_hnd_cert_req(ssl_common_dissect_t *hf, tvbuff_t *tvb, packet_info *pinfo,
                          proto_tree *tree, guint32 offset, guint32 offset_end,
-                         const SslSession *session);
+                         SslSession *session, gboolean is_dtls);
 
 extern void
 ssl_dissect_hnd_cli_cert_verify(ssl_common_dissect_t *hf, tvbuff_t *tvb,
diff --git a/epan/dissectors/packet-ssl.c b/epan/dissectors/packet-ssl.c
index ccb821728d..95ec9b1b57 100644
--- a/epan/dissectors/packet-ssl.c
+++ b/epan/dissectors/packet-ssl.c
@@ -2109,7 +2109,7 @@ 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, pinfo,
-                        ssl_hand_tree, offset, offset + length, session, ssl,
+                        ssl_hand_tree, offset, offset + length, session, ssl, FALSE,
                         ssl_master_key_map.tickets);
                 break;
 
@@ -2121,13 +2121,13 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
             case SSL_HND_ENCRYPTED_EXTENSIONS:
                 /* XXX expert info if used with non-TLS 1.3? */
                 ssl_dissect_hnd_encrypted_extensions(&dissect_ssl3_hf, tvb, pinfo, ssl_hand_tree,
-                                                     offset, length, session, ssl, FALSE);
+                                                     offset, offset + length, session, ssl, FALSE);
 
                 break;
 
             case SSL_HND_CERTIFICATE:
                 ssl_dissect_hnd_cert(&dissect_ssl3_hf, tvb, ssl_hand_tree,
-                        offset, offset + length, pinfo, session, ssl, ssl_key_hash, is_from_server);
+                        offset, offset + length, pinfo, session, ssl, ssl_key_hash, is_from_server, FALSE);
                 break;
 
             case SSL_HND_SERVER_KEY_EXCHG:
@@ -2135,7 +2135,7 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
                 break;
 
             case SSL_HND_CERT_REQUEST:
-                ssl_dissect_hnd_cert_req(&dissect_ssl3_hf, tvb, pinfo, ssl_hand_tree, offset, offset + length, session);
+                ssl_dissect_hnd_cert_req(&dissect_ssl3_hf, tvb, pinfo, ssl_hand_tree, offset, offset + length, session, FALSE);
                 break;
 
             case SSL_HND_SVR_HELLO_DONE: