osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

ssl,dtls: fix wrong expert info for overly large records 

The plaintext length is limited to 2^14, but the actual record length
(TLSCiphertext) may be larger due to expansion from compression and the
cipher (like AEAD auth tags). The wrong check led to false expert infos.

Change-Id: I3a56f1b0af05ecc1d97c4f1f0bcf35ff4d0fad42
Fixes: v2.3.0rc0-1584-gff0371e898 ("ssl,dtls: add expert info for overly large record lengths")
Reviewed-on: https://code.wireshark.org/review/20099
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Jaap Keuter <jaap.keuter@xs4all.nl>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
This commit is contained in:
Peter Wu 2017-02-14 11:18:24 +01:00
parent efcb5c07f0
commit bb1450b017
4 changed files with 35 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
epan/dissectors/packet-dtls.c
View File

@ -693,7 +693,7 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo,
guint8 next_byte;
proto_tree *ti;
proto_tree *dtls_record_tree;
proto_item *pi;
proto_item *length_pi;
tvbuff_t *decrypted;
SslRecordInfo *record = NULL;
heur_dtbl_entry_t *hdtbl_entry;
@ -761,11 +761,8 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo,
offset += 6;
/* add the length */
pi = proto_tree_add_uint(dtls_record_tree, hf_dtls_record_length, tvb,
length_pi = proto_tree_add_uint(dtls_record_tree, hf_dtls_record_length, tvb,
offset, 2, record_length);
if (record_length > TLS_MAX_RECORD_LENGTH) {
expert_add_info(pinfo, pi, &dissect_dtls_hf.ei.record_length_invalid);
}
offset += 2; /* move past length field itself */
/*
@ -794,6 +791,7 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo,
if (decrypted) {
add_new_data_source(pinfo, decrypted, "Decrypted DTLS");
}
ssl_check_record_length(&dissect_dtls_hf, pinfo, record_length, length_pi, session->version, decrypted);
switch ((ContentType) content_type) {

23
epan/dissectors/packet-ssl-utils.c
View File

@ -6669,6 +6669,29 @@ ssl_try_set_version(SslSession *session, SslDecryptSession *ssl,
}
}
void
ssl_check_record_length(ssl_common_dissect_t *hf, packet_info *pinfo,
guint record_length, proto_item *length_pi,
guint16 version, tvbuff_t *decrypted_tvb)
{
guint max_expansion;
if (version == TLSV1DOT3_VERSION) {
/* TLS 1.3: Max length is 2^14 + 256 */
max_expansion = 256;
} else {
/* RFC 5246, Section 6.2.3: TLSCiphertext.fragment length MUST NOT exceed 2^14 + 2048 */
max_expansion = 2048;
}
if (record_length > TLS_MAX_RECORD_LENGTH + max_expansion) {
expert_add_info_format(pinfo, length_pi, &hf->ei.record_length_invalid,
"TLSCiphertext length MUST NOT exceed 2^14 + %u", max_expansion);
}
if (decrypted_tvb && tvb_captured_length(decrypted_tvb) > TLS_MAX_RECORD_LENGTH) {
expert_add_info_format(pinfo, length_pi, &hf->ei.record_length_invalid,
"TLSPlaintext length MUST NOT exceed 2^14");
}
}
static void
ssl_set_cipher(SslDecryptSession *ssl, guint16 cipher)
{

7
epan/dissectors/packet-ssl-utils.h
View File

@ -870,6 +870,11 @@ ssl_end_vector(ssl_common_dissect_t *hf, tvbuff_t *tvb, packet_info *pinfo, prot
/* }}} */
extern void
ssl_check_record_length(ssl_common_dissect_t *hf, packet_info *pinfo,
guint record_length, proto_item *length_pi,
guint16 version, tvbuff_t *decrypted_tvb);
void
ssl_dissect_change_cipher_spec(ssl_common_dissect_t *hf, tvbuff_t *tvb,
packet_info *pinfo, proto_tree *tree,
@ -1641,7 +1646,7 @@ ssl_common_dissect_t name = { \
}, \
{ & name .ei.record_length_invalid, \
{ prefix ".record.length.invalid", PI_PROTOCOL, PI_ERROR, \
"Record fragment length must not exceed 2^14", EXPFILL } \
"Record fragment length is too large", EXPFILL } \
}
/* }}} */

8
epan/dissectors/packet-ssl.c
View File

@ -1546,7 +1546,7 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
guint8 next_byte;
proto_tree *ti;
proto_tree *ssl_record_tree;
proto_item *pi, *ct_pi;
proto_item *length_pi, *ct_pi;
guint content_type_offset;
guint32 available_bytes;
tvbuff_t *decrypted;
@ -1679,11 +1679,8 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
offset += 2;
/* add the length */
pi = proto_tree_add_uint(ssl_record_tree, hf_ssl_record_length, tvb,
length_pi = proto_tree_add_uint(ssl_record_tree, hf_ssl_record_length, tvb,
offset, 2, record_length);
if (record_length > TLS_MAX_RECORD_LENGTH) {
expert_add_info(pinfo, pi, &dissect_ssl3_hf.ei.record_length_invalid);
}
offset += 2; /* move past length field itself */
/*
@ -1739,6 +1736,7 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
PROTO_ITEM_SET_GENERATED(ti);
}
}
ssl_check_record_length(&dissect_ssl3_hf, pinfo, record_length, length_pi, version, decrypted);
switch ((ContentType) content_type) {
case SSL_ID_CHG_CIPHER_SPEC: