From 3f81af22e0116a2f83c0298de7874959b3967da2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stig=20Bj=C3=B8rlykke?= Date: Fri, 11 Apr 2014 13:13:42 +0200 Subject: Improved TLS/DTLS Heartbeat Message handling. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added an expert info for invalid payload length (heartbleed). Change-Id: I9e09d1ae2b997091bbce2483c098dba7e6631859 Reviewed-on: https://code.wireshark.org/review/1067 Reviewed-by: Stig Bjørlykke Tested-by: Stig Bjørlykke Reviewed-by: Michael Mann --- epan/dissectors/packet-dtls.c | 28 +++++++++++++++++++++------- epan/dissectors/packet-ssl.c | 29 ++++++++++++++++++++--------- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/epan/dissectors/packet-dtls.c b/epan/dissectors/packet-dtls.c index 6578f6e07b..64a3486c02 100644 --- a/epan/dissectors/packet-dtls.c +++ b/epan/dissectors/packet-dtls.c @@ -202,6 +202,7 @@ static expert_field ei_dtls_handshake_fragment_length_too_long = EI_INIT; static expert_field ei_dtls_handshake_fragment_past_end_msg = EI_INIT; static expert_field ei_dtls_msg_len_diff_fragment = EI_INIT; static expert_field ei_dtls_handshake_sig_hash_alg_len_bad = EI_INIT; +static expert_field ei_dtls_heartbeat_payload_length = EI_INIT; static GHashTable *dtls_session_hash = NULL; static GHashTable *dtls_key_hash = NULL; @@ -360,7 +361,8 @@ static void dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo, /* heartbeat message dissector */ static void dissect_dtls_heartbeat(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, guint32 offset, - guint *conv_version, guint32 record_length); + guint *conv_version, guint32 record_length, + gboolean decrypted); static void dissect_dtls_hnd_cli_hello(tvbuff_t *tvb, @@ -1073,11 +1075,11 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo, decrypted = ssl_get_record_info(tvb, proto_dtls, pinfo, offset); if (decrypted) { dissect_dtls_heartbeat(decrypted, pinfo, dtls_record_tree, 0, - conv_version, record_length); + conv_version, tvb_length (decrypted), TRUE); add_new_data_source(pinfo, decrypted, "Decrypted SSL record"); } else { dissect_dtls_heartbeat(tvb, pinfo, dtls_record_tree, offset, - conv_version, record_length); + conv_version, record_length, FALSE); } break; } @@ -1570,7 +1572,8 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo, static void dissect_dtls_heartbeat(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, guint32 offset, - guint* conv_version, guint32 record_length) + guint* conv_version, guint32 record_length, + gboolean decrypted) { /* struct { * HeartbeatMessageType type; @@ -1610,7 +1613,7 @@ dissect_dtls_heartbeat(tvbuff_t *tvb, packet_info *pinfo, } if (tree) { - if (type && (payload_length <= record_length - 16 - 3)) { + if (type && ((payload_length <= record_length - 16 - 3) || decrypted)) { proto_item_set_text(tree, "%s Record Layer: Heartbeat " "%s", val_to_str_const(*conv_version, ssl_version_short_names, "SSL"), @@ -1618,9 +1621,18 @@ dissect_dtls_heartbeat(tvbuff_t *tvb, packet_info *pinfo, proto_tree_add_item(dtls_heartbeat_tree, hf_dtls_heartbeat_message_type, tvb, offset, 1, ENC_BIG_ENDIAN); offset += 1; - proto_tree_add_uint(dtls_heartbeat_tree, hf_dtls_heartbeat_message_payload_length, - tvb, offset, 2, payload_length); + ti = proto_tree_add_uint(dtls_heartbeat_tree, hf_dtls_heartbeat_message_payload_length, + tvb, offset, 2, payload_length); offset += 2; + if (payload_length > record_length - 16 - 3) { + expert_add_info_format(pinfo, ti, &ei_dtls_heartbeat_payload_length, + "Invalid heartbeat payload length (%d)", payload_length); + /* Invalid heartbeat payload length, adjust to try decoding */ + payload_length = record_length - 16 - 3; + padding_length = 16; + proto_item_append_text (ti, " (invalid, using %u to decode payload)", payload_length); + + } proto_tree_add_bytes_format(dtls_heartbeat_tree, hf_dtls_heartbeat_message_payload, tvb, offset, payload_length, NULL, "Payload (%u byte%s)", @@ -3326,6 +3338,8 @@ proto_register_dtls(void) { &ei_dtls_handshake_fragment_past_end_msg, { "dtls.handshake.fragment_past_end_msg", PI_PROTOCOL, PI_ERROR, "Fragment runs past the end of the message", EXPFILL }}, { &ei_dtls_msg_len_diff_fragment, { "dtls.msg_len_diff_fragment", PI_PROTOCOL, PI_ERROR, "Message length differs from value in earlier fragment", EXPFILL }}, { &ei_dtls_handshake_sig_hash_alg_len_bad, { "dtls.handshake.sig_hash_alg_len.bad", PI_MALFORMED, PI_ERROR, "Signature Hash Algorithm length must be a multiple of 2", EXPFILL }}, + { &ei_dtls_heartbeat_payload_length, {"dtls.heartbeat_message.payload_length.invalid", PI_MALFORMED, PI_ERROR, "Invalid heartbeat payload length", EXPFILL }}, + SSL_COMMON_EI_LIST(dissect_dtls_hf, "dtls") }; diff --git a/epan/dissectors/packet-ssl.c b/epan/dissectors/packet-ssl.c index 81994edffd..4a8ba03933 100644 --- a/epan/dissectors/packet-ssl.c +++ b/epan/dissectors/packet-ssl.c @@ -284,7 +284,7 @@ static gint ett_ssl_segment = -1; static expert_field ei_ssl_handshake_cipher_suites_mult2 = EI_INIT; static expert_field ei_ssl_handshake_sig_hash_algs_mult2 = EI_INIT; static expert_field ei_ssl2_handshake_session_id_len_error = EI_INIT; - +static expert_field ei_ssl3_heartbeat_payload_length = EI_INIT; /* not all of the hf_fields below make sense for SSL but we have to provide them anyways to comply with the api (which was aimed for ip fragment @@ -522,7 +522,8 @@ static void dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo, /* heartbeat message dissector */ static void dissect_ssl3_heartbeat(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, guint32 offset, - guint *conv_version, guint32 record_length); + guint *conv_version, guint32 record_length, + gboolean decrypted); static void dissect_ssl3_hnd_cli_hello(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, @@ -1765,9 +1766,9 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo, decrypted = ssl_get_record_info(tvb, proto_ssl, pinfo, offset); if (decrypted) { add_new_data_source(pinfo, decrypted, "Decrypted SSL record"); - dissect_ssl3_heartbeat(decrypted, pinfo, ssl_record_tree, 0, conv_version, record_length); + dissect_ssl3_heartbeat(decrypted, pinfo, ssl_record_tree, 0, conv_version, tvb_length (decrypted), TRUE); } else { - dissect_ssl3_heartbeat(tvb, pinfo, ssl_record_tree, offset, conv_version, record_length); + dissect_ssl3_heartbeat(tvb, pinfo, ssl_record_tree, offset, conv_version, record_length, FALSE); } break; } @@ -2140,7 +2141,8 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo, static void dissect_ssl3_heartbeat(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, guint32 offset, - guint *conv_version, guint32 record_length) + guint *conv_version, guint32 record_length, + gboolean decrypted) { /* struct { * HeartbeatMessageType type; @@ -2177,14 +2179,14 @@ dissect_ssl3_heartbeat(tvbuff_t *tvb, packet_info *pinfo, padding_length = record_length - 3 - payload_length; /* now set the text in the record layer line */ - if (type && (payload_length <= record_length - 16 - 3)) { + if (type && ((payload_length <= record_length - 16 - 3) || decrypted)) { col_append_fstr(pinfo->cinfo, COL_INFO, "Heartbeat %s", type); } else { col_append_str(pinfo->cinfo, COL_INFO, "Encrypted Heartbeat"); } if (tree) { - if (type && (payload_length <= record_length - 16 - 3)) { + if (type && ((payload_length <= record_length - 16 - 3) || decrypted)) { proto_item_set_text(tree, "%s Record Layer: Heartbeat " "%s", val_to_str_const(*conv_version, ssl_version_short_names, "SSL"), @@ -2192,9 +2194,17 @@ dissect_ssl3_heartbeat(tvbuff_t *tvb, packet_info *pinfo, proto_tree_add_item(tls_heartbeat_tree, hf_ssl_heartbeat_message_type, tvb, offset, 1, ENC_BIG_ENDIAN); offset += 1; - proto_tree_add_uint(tls_heartbeat_tree, hf_ssl_heartbeat_message_payload_length, - tvb, offset, 2, payload_length); + ti = proto_tree_add_uint(tls_heartbeat_tree, hf_ssl_heartbeat_message_payload_length, + tvb, offset, 2, payload_length); offset += 2; + if (payload_length > record_length - 16 - 3) { + expert_add_info_format(pinfo, ti, &ei_ssl3_heartbeat_payload_length, + "Invalid payload heartbeat length (%d)", payload_length); + /* Invalid heartbeat payload length, adjust to try decoding */ + payload_length = record_length - 16 - 3; + padding_length = 16; + proto_item_append_text (ti, " (invalid, using %u to decode payload)", payload_length); + } proto_tree_add_bytes_format(tls_heartbeat_tree, hf_ssl_heartbeat_message_payload, tvb, offset, payload_length, NULL, "Payload (%u byte%s)", @@ -5521,6 +5531,7 @@ proto_register_ssl(void) { &ei_ssl_handshake_cipher_suites_mult2, { "ssl.handshake.cipher_suites_length.mult2", PI_MALFORMED, PI_ERROR, "Cipher suite length must be a multiple of 2", EXPFILL }}, { &ei_ssl_handshake_sig_hash_algs_mult2, { "ssl.handshake.sig_hash_alg_len.mult2", PI_MALFORMED, PI_ERROR, "Signature Hash Algorithm length must be a multiple of 2", EXPFILL }}, { &ei_ssl2_handshake_session_id_len_error, { "ssl.handshake.session_id_length.error", PI_MALFORMED, PI_ERROR, "Session ID length error", EXPFILL }}, + { &ei_ssl3_heartbeat_payload_length, {"ssl.heartbeat_message.payload_length.invalid", PI_MALFORMED, PI_ERROR, "Invalid heartbeat payload length", EXPFILL }}, SSL_COMMON_EI_LIST(dissect_ssl3_hf, "ssl") }; -- cgit v1.2.3