From 74436b5ace977279b659dc2420305ea5a423e9ee Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Thu, 24 Aug 2017 00:36:03 -0700 Subject: ssl: fix subdissection with multiple TLS records per packet Decrypted TLS records must be stored in a single SslPacketInfo or else plaintext will go missing (in Follow SSL or when subdissectors need reassembly). As this structure is currently keyed by the layer number (pinfo->curr_layer_num) which is changed by call_dissector, it must be copied and propagated before calling subdissectors. Change-Id: Ic42ba6c0854154272058f9bf9796e06ad7f94bfd Fixes: v2.3.0rc0-3740-ge1f84f985e ("Fix Decode As for protocols that may use tunneling.") Bug: 13885 Reviewed-on: https://code.wireshark.org/review/23190 Reviewed-by: Peter Wu Petri-Dish: Peter Wu Tested-by: Petri Dish Buildbot Reviewed-by: Michael Mann --- epan/dissectors/packet-dtls.c | 20 ++++++++++++-------- epan/dissectors/packet-ssl-utils.c | 11 ++++++----- epan/dissectors/packet-ssl-utils.h | 4 ++-- epan/dissectors/packet-ssl.c | 20 ++++++++++++-------- 4 files changed, 32 insertions(+), 23 deletions(-) diff --git a/epan/dissectors/packet-dtls.c b/epan/dissectors/packet-dtls.c index 75dfbd2ddc..ccdf542797 100644 --- a/epan/dissectors/packet-dtls.c +++ b/epan/dissectors/packet-dtls.c @@ -323,7 +323,8 @@ dtls_parse_old_keys(void) static gint dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, guint32 offset, SslSession *session, gint is_from_server, - SslDecryptSession *conv_data); + SslDecryptSession *conv_data, + guint8 curr_layer_num_ssl); /* alert message dissector */ static void dissect_dtls_alert(tvbuff_t *tvb, packet_info *pinfo, @@ -374,6 +375,7 @@ dissect_dtls(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_ SslDecryptSession *ssl_session; SslSession *session; gint is_from_server; + guint8 curr_layer_num_ssl = pinfo->curr_layer_num; ti = NULL; dtls_tree = NULL; @@ -440,7 +442,7 @@ dissect_dtls(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_ case DTLSV1DOT2_VERSION: offset = dissect_dtls_record(tvb, pinfo, dtls_tree, offset, session, is_from_server, - ssl_session); + ssl_session, curr_layer_num_ssl); break; /* that failed, so apply some heuristics based @@ -452,7 +454,7 @@ dissect_dtls(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_ /* looks like dtls */ offset = dissect_dtls_record(tvb, pinfo, dtls_tree, offset, session, is_from_server, - ssl_session); + ssl_session, curr_layer_num_ssl); } else { @@ -473,6 +475,7 @@ dissect_dtls(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_ first_record_in_frame = FALSE; } + // XXX there is no Follow DTLS Stream, is this tap needed? tap_queue_packet(dtls_tap, pinfo, NULL); return tvb_captured_length(tvb); } @@ -568,7 +571,7 @@ dtls_is_null_cipher(guint cipher ) static gboolean decrypt_dtls_record(tvbuff_t *tvb, packet_info *pinfo, guint32 offset, SslDecryptSession *ssl, - guint8 content_type, guint16 record_version, guint16 record_length) + guint8 content_type, guint16 record_version, guint16 record_length, guint8 curr_layer_num_ssl) { gboolean success; SslDecoder *decoder; @@ -635,7 +638,7 @@ decrypt_dtls_record(tvbuff_t *tvb, packet_info *pinfo, guint32 offset, SslDecryp ssl_add_record_info(proto_dtls, pinfo, data, datalen, tvb_raw_offset(tvb)+offset, - NULL, (ContentType)content_type); + NULL, (ContentType)content_type, curr_layer_num_ssl); } return success; } @@ -662,7 +665,8 @@ static gint dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, guint32 offset, SslSession *session, gint is_from_server, - SslDecryptSession* ssl) + SslDecryptSession* ssl, + guint8 curr_layer_num_ssl) { /* @@ -786,9 +790,9 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo, /* try to decrypt record on the first pass, if possible. Store decrypted * record for later usage (without having to decrypt again). */ if (ssl) { - decrypt_dtls_record(tvb, pinfo, offset, ssl, content_type, version, record_length); + decrypt_dtls_record(tvb, pinfo, offset, ssl, content_type, version, record_length, curr_layer_num_ssl); } - decrypted = ssl_get_record_info(tvb, proto_dtls, pinfo, tvb_raw_offset(tvb)+offset, &record); + decrypted = ssl_get_record_info(tvb, proto_dtls, pinfo, tvb_raw_offset(tvb)+offset, curr_layer_num_ssl, &record); if (decrypted) { add_new_data_source(pinfo, decrypted, "Decrypted DTLS"); } diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c index 325a751c5c..64f8709bd8 100644 --- a/epan/dissectors/packet-ssl-utils.c +++ b/epan/dissectors/packet-ssl-utils.c @@ -4527,20 +4527,21 @@ ssl_packet_from_server(SslSession *session, dissector_table_t table, packet_info * @param record_id The identifier for this record within the current packet. * @param flow Information about sequence numbers, etc. * @param type TLS Content Type (such as handshake or application_data). + * @param curr_layer_num_ssl The layer identifier for this TLS session. */ void -ssl_add_record_info(gint proto, packet_info *pinfo, const guchar *data, gint data_len, gint record_id, SslFlow *flow, ContentType type) +ssl_add_record_info(gint proto, packet_info *pinfo, const guchar *data, gint data_len, gint record_id, SslFlow *flow, ContentType type, guint8 curr_layer_num_ssl) { SslRecordInfo* rec, **prec; SslPacketInfo* pi; - pi = (SslPacketInfo *)p_get_proto_data(wmem_file_scope(), pinfo, proto, pinfo->curr_layer_num); + pi = (SslPacketInfo *)p_get_proto_data(wmem_file_scope(), pinfo, proto, curr_layer_num_ssl); if (!pi) { pi = wmem_new0(wmem_file_scope(), SslPacketInfo); pi->srcport = pinfo->srcport; pi->destport = pinfo->destport; - p_add_proto_data(wmem_file_scope(), pinfo, proto, pinfo->curr_layer_num, pi); + p_add_proto_data(wmem_file_scope(), pinfo, proto, curr_layer_num_ssl, pi); } rec = wmem_new(wmem_file_scope(), SslRecordInfo); @@ -4569,11 +4570,11 @@ ssl_add_record_info(gint proto, packet_info *pinfo, const guchar *data, gint dat /* search in packet data for the specified id; return a newly created tvb for the associated data */ tvbuff_t* -ssl_get_record_info(tvbuff_t *parent_tvb, int proto, packet_info *pinfo, gint record_id, SslRecordInfo **matched_record) +ssl_get_record_info(tvbuff_t *parent_tvb, int proto, packet_info *pinfo, gint record_id, guint8 curr_layer_num_ssl, SslRecordInfo **matched_record) { SslRecordInfo* rec; SslPacketInfo* pi; - pi = (SslPacketInfo *)p_get_proto_data(wmem_file_scope(), pinfo, proto, pinfo->curr_layer_num); + pi = (SslPacketInfo *)p_get_proto_data(wmem_file_scope(), pinfo, proto, curr_layer_num_ssl); if (!pi) return NULL; diff --git a/epan/dissectors/packet-ssl-utils.h b/epan/dissectors/packet-ssl-utils.h index f20ea7d5c5..c0d7551ce4 100644 --- a/epan/dissectors/packet-ssl-utils.h +++ b/epan/dissectors/packet-ssl-utils.h @@ -632,11 +632,11 @@ ssl_packet_from_server(SslSession *session, dissector_table_t table, packet_info /* add to packet data a copy of the specified real data */ extern void -ssl_add_record_info(gint proto, packet_info *pinfo, const guchar *data, gint data_len, gint record_id, SslFlow *flow, ContentType type); +ssl_add_record_info(gint proto, packet_info *pinfo, const guchar *data, gint data_len, gint record_id, SslFlow *flow, ContentType type, guint8 curr_layer_num_ssl); /* search in packet data for the specified id; return a newly created tvb for the associated data */ extern tvbuff_t* -ssl_get_record_info(tvbuff_t *parent_tvb, gint proto, packet_info *pinfo, gint record_id, SslRecordInfo **matched_record); +ssl_get_record_info(tvbuff_t *parent_tvb, gint proto, packet_info *pinfo, gint record_id, guint8 curr_layer_num_ssl, SslRecordInfo **matched_record); /* initialize/reset per capture state data (ssl sessions cache) */ extern void diff --git a/epan/dissectors/packet-ssl.c b/epan/dissectors/packet-ssl.c index 848fff1ef3..3cf4ab2c9b 100644 --- a/epan/dissectors/packet-ssl.c +++ b/epan/dissectors/packet-ssl.c @@ -548,7 +548,8 @@ static gint dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, guint32 offset, SslSession *session, gint is_from_server, gboolean *need_desegmentation, - SslDecryptSession *conv_data); + SslDecryptSession *conv_data, + guint8 curr_layer_num_ssl); /* alert message dissector */ static void dissect_ssl3_alert(tvbuff_t *tvb, packet_info *pinfo, @@ -803,7 +804,8 @@ dissect_ssl(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data _U_) offset = dissect_ssl3_record(tvb, pinfo, ssl_tree, offset, session, is_from_server, &need_desegmentation, - ssl_session); + ssl_session, + curr_layer_num_ssl); } break; @@ -825,7 +827,8 @@ dissect_ssl(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data _U_) offset = dissect_ssl3_record(tvb, pinfo, ssl_tree, offset, session, is_from_server, &need_desegmentation, - ssl_session); + ssl_session, + curr_layer_num_ssl); } else { @@ -980,7 +983,7 @@ dissect_ssl_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data static gboolean decrypt_ssl3_record(tvbuff_t *tvb, packet_info *pinfo, guint32 offset, SslDecryptSession *ssl, guint8 content_type, guint16 record_version, guint16 record_length, - gboolean allow_fragments) + gboolean allow_fragments, guint8 curr_layer_num_ssl) { gboolean success; gint direction; @@ -1055,7 +1058,7 @@ decrypt_ssl3_record(tvbuff_t *tvb, packet_info *pinfo, guint32 offset, SslDecryp * Alert messages MUST NOT be fragmented across records, so do not * bother maintaining a flow for those. */ ssl_add_record_info(proto_ssl, pinfo, data, datalen, tvb_raw_offset(tvb)+offset, - allow_fragments ? decoder->flow : NULL, (ContentType)content_type); + allow_fragments ? decoder->flow : NULL, (ContentType)content_type, curr_layer_num_ssl); } return success; } @@ -1616,7 +1619,8 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, guint32 offset, SslSession *session, gint is_from_server, gboolean *need_desegmentation, - SslDecryptSession *ssl) + SslDecryptSession *ssl, + guint8 curr_layer_num_ssl) { /* @@ -1798,10 +1802,10 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo, decrypt_ssl3_record(tvb, pinfo, offset, ssl, content_type, record_version, record_length, content_type == SSL_ID_APP_DATA || - content_type == SSL_ID_HANDSHAKE); + content_type == SSL_ID_HANDSHAKE, curr_layer_num_ssl); } /* try to retrieve and use decrypted alert/handshake/appdata record, if any. */ - decrypted = ssl_get_record_info(tvb, proto_ssl, pinfo, tvb_raw_offset(tvb)+offset, &record); + decrypted = ssl_get_record_info(tvb, proto_ssl, pinfo, tvb_raw_offset(tvb)+offset, curr_layer_num_ssl, &record); if (decrypted) { add_new_data_source(pinfo, decrypted, "Decrypted SSL"); if (session->version == TLSV1DOT3_VERSION) { -- cgit v1.2.3