diff options
| author | Andre Luyer <andre@luyer.nl> | 2020-03-06 16:19:46 +0100 |
|---|---|---|
| committer | Anders Broman <a.broman58@gmail.com> | 2020-03-09 04:42:30 +0000 |
| commit | 914bb159f781c2f3a788fb68a3bacd3f54e24179 (patch) | |
| tree | 9d642a06f0c14f3523c027d418c372cb65d82325 /epan | |
| parent | 839209d2198cc06ca768dcb5e428ca6b21974b25 (diff) | |
TLS: Limit the number of DNs in Certificate Request messages
When the Distinguished Names list is large in the Certificate Request it can
trigger the "Dissector bug warning". Having a huge list in the tree pane is
not useful anyway so this change limits the amount of DNs added to the tree,
preventing this fault condition.
It is indicated by adding a "[Tree view truncated]" item.
A side effect is that a handshake is no longer incorrectly flagged as 'resumed'
because the Server Hello Done in the same packet is now dissected.
Bug: 16202
Change-Id: Ib315940dcabc2d6b31cf3562354214158ea545a5
Reviewed-on: https://code.wireshark.org/review/36314
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Diffstat (limited to 'epan')
| -rw-r--r-- | epan/dissectors/packet-tls-utils.c | 15 | ||||
| -rw-r--r-- | epan/dissectors/packet-tls-utils.h | 8 |
2 files changed, 21 insertions, 2 deletions
diff --git a/epan/dissectors/packet-tls-utils.c b/epan/dissectors/packet-tls-utils.c index a01214ee94..67a59bae69 100644 --- a/epan/dissectors/packet-tls-utils.c +++ b/epan/dissectors/packet-tls-utils.c @@ -6028,7 +6028,7 @@ tls_dissect_certificate_authorities(ssl_common_dissect_t *hf, tvbuff_t *tvb, pac proto_tree *subtree; guint32 dnames_length, next_offset; asn1_ctx_t asn1_ctx; - + int dnames_count = 100; /* the maximum number of DNs to add to the tree */ /* Note: minimum length is 0 for TLS 1.1/1.2 and 3 for earlier/later */ /* DistinguishedName certificate_authorities<0..2^16-1> */ @@ -6053,6 +6053,19 @@ tls_dissect_certificate_authorities(ssl_common_dissect_t *hf, tvbuff_t *tvb, pac while (offset < next_offset) { /* get the length of the current certificate */ guint32 name_length; + + if (dnames_count-- == 0) { + /* stop adding to tree when the list is considered too large + * https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=16202 + Note: dnames_count must be set low enough not to hit the + limit set by PINFO_LAYER_MAX_RECURSION_DEPTH in packet.c + */ + ti = proto_tree_add_item(subtree, hf->hf.hs_dnames_truncated, + tvb, offset, next_offset - offset, ENC_NA); + proto_item_set_generated(ti); + return next_offset; + } + /* opaque DistinguishedName<1..2^16-1> */ if (!ssl_add_vector(hf, tvb, pinfo, subtree, offset, next_offset, &name_length, hf->hf.hs_dname_len, 1, G_MAXUINT16)) { diff --git a/epan/dissectors/packet-tls-utils.h b/epan/dissectors/packet-tls-utils.h index c338e73535..23b642a4f1 100644 --- a/epan/dissectors/packet-tls-utils.h +++ b/epan/dissectors/packet-tls-utils.h @@ -839,6 +839,7 @@ typedef struct ssl_common_dissect { gint hs_cert_type; gint hs_dnames_len; gint hs_dnames; + gint hs_dnames_truncated; gint hs_dname_len; gint hs_dname; gint hs_random; @@ -1152,7 +1153,7 @@ ssl_common_dissect_t name = { \ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, \ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, \ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, \ - -1, -1, -1, -1, -1, -1, -1, \ + -1, -1, -1, -1, -1, -1, -1, -1, \ }, \ /* ett */ { \ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, \ @@ -1666,6 +1667,11 @@ ssl_common_dissect_t name = { \ FT_UINT16, BASE_DEC, NULL, 0x0, \ "Length of distinguished name", HFILL } \ }, \ + { & name .hf.hs_dnames_truncated, \ + { "Tree view truncated", prefix ".handshake.dnames_truncated", \ + FT_NONE, BASE_NONE, NULL, 0x00, \ + "Some Distinguished Names are not added to tree pane to limit resources", HFILL } \ + }, \ { & name .hf.hs_dname, \ { "Distinguished Name", prefix ".handshake.dname", \ FT_NONE, BASE_NONE, NULL, 0x0, \ |