From 9d189c7e20de37f95b2ad70725ab65b9bf863227 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Sat, 14 Oct 2017 18:43:58 +0100 Subject: ssl: assume everything after CCS is encrypted After ChangeCipherSpec, record fragments are encrypted. Use this strong hint to fix misinterpreting the explicit nonce as a handshake message. One edge case remaing unsolved though, if an encrypted Finished message follows the CCS in the same TCP packet, then it could still be misinterpreted. Bug: 14117 Change-Id: Ie54bb5335f115d0fd8f05a13d1c826e3807cbbd3 Reviewed-on: https://code.wireshark.org/review/23900 Petri-Dish: Peter Wu Tested-by: Petri Dish Buildbot Reviewed-by: Alexis La Goutte --- epan/dissectors/packet-ssl-utils.c | 5 +++++ epan/dissectors/packet-ssl-utils.h | 2 ++ epan/dissectors/packet-ssl.c | 9 +++++++++ 3 files changed, 16 insertions(+) diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c index 7b3683d9cc..5d0172a590 100644 --- a/epan/dissectors/packet-ssl-utils.c +++ b/epan/dissectors/packet-ssl-utils.c @@ -5565,6 +5565,11 @@ ssl_dissect_change_cipher_spec(ssl_common_dissect_t *hf, tvbuff_t *tvb, val_to_str_const(SSL_ID_CHG_CIPHER_SPEC, ssl_31_content_type, "unknown")); ti = proto_tree_add_item(tree, hf->hf.change_cipher_spec, tvb, offset, 1, ENC_NA); + /* Remember frame number of first CCS */ + guint32 *ccs_frame = is_from_server ? &session->server_ccs_frame : &session->client_ccs_frame; + if (*ccs_frame == 0) + *ccs_frame = pinfo->num; + /* Use heuristics to detect an abbreviated handshake, assume that missing * ServerHelloDone implies reusing previously negotiating keys. Then when * a Session ID or ticket is present, it must be a resumed session. diff --git a/epan/dissectors/packet-ssl-utils.h b/epan/dissectors/packet-ssl-utils.h index 4fe57baadd..edf070e741 100644 --- a/epan/dissectors/packet-ssl-utils.h +++ b/epan/dissectors/packet-ssl-utils.h @@ -430,6 +430,8 @@ typedef struct _SslSession { guchar tls13_draft_version; gint8 client_cert_type; gint8 server_cert_type; + guint32 client_ccs_frame; + guint32 server_ccs_frame; /* The address/proto/port of the server as determined from heuristics * (e.g. ClientHello) or set externally (via ssl_set_master_secret()). */ diff --git a/epan/dissectors/packet-ssl.c b/epan/dissectors/packet-ssl.c index c0e4233387..87983e843e 100644 --- a/epan/dissectors/packet-ssl.c +++ b/epan/dissectors/packet-ssl.c @@ -2037,6 +2037,15 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo, if (maybe_encrypted) { maybe_encrypted = tvb_bytes_exist(tvb, offset, 5) && tvb_get_ntoh40(tvb, offset) == 0; } + /* + * Everything after the ChangeCipherSpec message is encrypted. + * TODO handle Finished message after CCS in the same frame and remove the + * above nonce-based heuristic. + */ + if (!maybe_encrypted) { + guint32 ccs_frame = is_from_server ? session->server_ccs_frame : session->client_ccs_frame; + maybe_encrypted = ccs_frame != 0 && pinfo->num > ccs_frame; + } /* just as there can be multiple records per packet, there * can be multiple messages per record as long as they have -- cgit v1.2.3