ssl: fix decryption when session ticket is not used

Do not use the client-supplied session ticket for decryption when the
session is not resumed as the cached key (associated with that ticket)
is invalid for this new session. SSL Session IDs are unaffected by this
issue as only the server-issued Session ID is considered.

This fixes decryption of a SSL capture which uses the keylog file for
decryption, but where the session tickets are invalid because the server
was restarted.

Additionally, the session and session tickets stores are split to avoid
exporting session tickets via File -> Export SSL Session keys. Session
tickets should only be used internally, the CLIENT_RANDOM identifier is
shorter and is the preferred method to link secrets.

Change-Id: If96d7a4e89389825478e67e9a65401ce0607aa66
Reviewed-on: https://code.wireshark.org/review/13994
Reviewed-by: Michael Mann <mmann78@netscape.net>

1 files changed, 12 insertions, 4 deletions

diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c
index b223eae248..7d7397cdbc 100644
--- a/epan/dissectors/packet-ssl-utils.c
+++ b/epan/dissectors/packet-ssl-utils.c
@@ -4344,6 +4344,7 @@ ssl_common_init(ssl_master_key_map_t *mk_map,
                 StringInfo *decrypted_data, StringInfo *compressed_data)
 {
     mk_map->session = g_hash_table_new(ssl_hash, ssl_equal);
+    mk_map->tickets = g_hash_table_new(ssl_hash, ssl_equal);
     mk_map->crandom = g_hash_table_new(ssl_hash, ssl_equal);
     mk_map->pre_master = g_hash_table_new(ssl_hash, ssl_equal);
     mk_map->pms = g_hash_table_new(ssl_hash, ssl_equal);
@@ -4356,6 +4357,7 @@ ssl_common_cleanup(ssl_master_key_map_t *mk_map, FILE **ssl_keylog_file,
                    StringInfo *decrypted_data, StringInfo *compressed_data)
 {
     g_hash_table_destroy(mk_map->session);
+    g_hash_table_destroy(mk_map->tickets);
     g_hash_table_destroy(mk_map->crandom);
     g_hash_table_destroy(mk_map->pre_master);
     g_hash_table_destroy(mk_map->pms);
@@ -4543,8 +4545,9 @@ ssl_finalize_decryption(SslDecryptSession *ssl, ssl_master_key_map_t *mk_map)
     if (!(ssl->state & (SSL_MASTER_SECRET | SSL_PRE_MASTER_SECRET)) &&
         !ssl_restore_master_key(ssl, "Session ID", FALSE,
                                 mk_map->session, &ssl->session_id) &&
-        !ssl_restore_master_key(ssl, "Session Ticket", FALSE,
-                                mk_map->session, &ssl->session_ticket) &&
+        (!ssl->session.is_session_resumed ||
+         !ssl_restore_master_key(ssl, "Session Ticket", FALSE,
+                                 mk_map->tickets, &ssl->session_ticket)) &&
         !ssl_restore_master_key(ssl, "Client Random", FALSE,
                                 mk_map->crandom, &ssl->client_random)) {
         if (ssl->cipher_suite.enc != ENC_NULL) {
@@ -4566,8 +4569,12 @@ ssl_finalize_decryption(SslDecryptSession *ssl, ssl_master_key_map_t *mk_map)
                         &ssl->client_random, &ssl->master_secret);
     ssl_save_master_key("Session ID", mk_map->session,
                         &ssl->session_id, &ssl->master_secret);
-    ssl_save_master_key("Session Ticket", mk_map->session,
-                        &ssl->session_ticket, &ssl->master_secret);
+    /* Only save the new secrets if the server sent the ticket. The client
+     * ticket might have become stale. */
+    if (ssl->state & SSL_NEW_SESSION_TICKET) {
+        ssl_save_master_key("Session Ticket", mk_map->tickets,
+                            &ssl->session_ticket, &ssl->master_secret);
+    }
 } /* }}} */
 #endif /* HAVE_LIBGCRYPT */
 
@@ -5906,6 +5913,7 @@ ssl_dissect_hnd_new_ses_ticket(ssl_common_dissect_t *hf, tvbuff_t *tvb,
          * master key (from the first CCS), save the ticket here too. */
         ssl_save_master_key("Session Ticket", session_hash,
                             &ssl->session_ticket, &ssl->master_secret);
+        ssl->state |= SSL_NEW_SESSION_TICKET;
     }
 #endif
 } /* }}} */