osmocom
/
wireshark
14
0
Fork 1
Code Issues Pull Requests Projects Releases Wiki Activity

(D)TLS: simplify SignatureAndHashAlgorithm dissection 

Merge the length parsing into the SignatureAndHashAlgorithm vector
parsing. Remove extra expert info which are replaced by the generic
ones.

Tested with a mutated pcap where the signature length field is off by
one (too large = expert error, too small = expert warning, as expected).

Change-Id: I43350352ae00eb42bbe5c2ee81289fb592b88f86
Reviewed-on: https://code.wireshark.org/review/19933
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
This commit is contained in:
Peter Wu 2017-02-03 15:15:10 +01:00 committed by Alexis La Goutte
parent 4623b05cd5
commit 658253ba34
4 changed files with 36 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1299,7 +1299,7 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo,
break; break;
case SSL_HND_CERT_REQUEST: case SSL_HND_CERT_REQUEST:
ssl_dissect_hnd_cert_req(&dissect_dtls_hf, sub_tvb, ssl_hand_tree, 0, pinfo, session); ssl_dissect_hnd_cert_req(&dissect_dtls_hf, sub_tvb, pinfo, ssl_hand_tree, 0, length, session);
break; break;
case SSL_HND_SVR_HELLO_DONE: case SSL_HND_SVR_HELLO_DONE:

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

@ -5752,29 +5752,34 @@ ssl_dissect_change_cipher_spec(ssl_common_dissect_t *hf, tvbuff_t *tvb,
TLS1.2 certificate request. {{{ */ TLS1.2 certificate request. {{{ */
static gint static gint
ssl_dissect_hash_alg_list(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tree, ssl_dissect_hash_alg_list(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tree,
packet_info* pinfo, guint32 offset, guint16 len) packet_info* pinfo, guint32 offset, guint32 offset_end)
{ {
guint32 offset_start; /* https://tools.ietf.org/html/rfc5246#section-7.4.1.4.1
* struct {
* HashAlgorithm hash;
* SignatureAlgorithm signature;
* } SignatureAndHashAlgorithm;
* SignatureAndHashAlgorithm supported_signature_algorithms<2..2^16-2>;
*/
proto_tree *subtree, *alg_tree; proto_tree *subtree, *alg_tree;
proto_item *ti; proto_item *ti;
guint sh_alg_length;
guint32 next_offset;
offset_start = offset; /* SignatureAndHashAlgorithm supported_signature_algorithms<2..2^16-2> */
if (len==0) if (!ssl_add_vector(hf, tvb, pinfo, tree, offset, offset_end, &sh_alg_length,
return 0; hf->hf.hs_sig_hash_alg_len, 2, G_MAXUINT16 - 1)) {
return offset_end;
}
offset += 2;
next_offset = offset + sh_alg_length;
ti = proto_tree_add_none_format(tree, hf->hf.hs_sig_hash_algs, tvb, ti = proto_tree_add_none_format(tree, hf->hf.hs_sig_hash_algs, tvb, offset, sh_alg_length,
offset, len,
"Signature Hash Algorithms (%u algorithm%s)", "Signature Hash Algorithms (%u algorithm%s)",
len / 2, plurality(len / 2, "", "s")); sh_alg_length / 2, plurality(sh_alg_length / 2, "", "s"));
subtree = proto_item_add_subtree(ti, hf->ett.hs_sig_hash_algs); subtree = proto_item_add_subtree(ti, hf->ett.hs_sig_hash_algs);
if (len % 2) { while (offset + 2 <= next_offset) {
expert_add_info_format(pinfo, ti, &hf->ei.hs_sig_hash_algs_bad,
"Invalid Signature Hash Algorithm length: %d", len);
return offset-offset_start;
}
while (len > 0) {
ti = proto_tree_add_item(subtree, hf->hf.hs_sig_hash_alg, ti = proto_tree_add_item(subtree, hf->hf.hs_sig_hash_alg,
tvb, offset, 2, ENC_BIG_ENDIAN); tvb, offset, 2, ENC_BIG_ENDIAN);
alg_tree = proto_item_add_subtree(ti, hf->ett.hs_sig_hash_alg); alg_tree = proto_item_add_subtree(ti, hf->ett.hs_sig_hash_alg);
@ -5785,9 +5790,13 @@ ssl_dissect_hash_alg_list(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *t
tvb, offset+1, 1, ENC_BIG_ENDIAN); tvb, offset+1, 1, ENC_BIG_ENDIAN);
offset += 2; offset += 2;
len -= 2;
} }
return offset-offset_start;
if (!ssl_end_vector(hf, tvb, pinfo, subtree, offset, next_offset)) {
offset = next_offset;
}
return offset;
} /* }}} */ } /* }}} */
/** TLS Extensions (in Client Hello and Server Hello). {{{ */ /** TLS Extensions (in Client Hello and Server Hello). {{{ */
@ -5795,24 +5804,7 @@ static gint
ssl_dissect_hnd_hello_ext_sig_hash_algs(ssl_common_dissect_t *hf, tvbuff_t *tvb, ssl_dissect_hnd_hello_ext_sig_hash_algs(ssl_common_dissect_t *hf, tvbuff_t *tvb,
proto_tree *tree, packet_info* pinfo, guint32 offset, guint32 offset_end) proto_tree *tree, packet_info* pinfo, guint32 offset, guint32 offset_end)
{ {
guint16 sh_alg_length; return ssl_dissect_hash_alg_list(hf, tvb, tree, pinfo, offset, offset_end);
gint ret;
guint ext_len = offset_end - offset;
sh_alg_length = tvb_get_ntohs(tvb, offset);
proto_tree_add_uint(tree, hf->hf.hs_sig_hash_alg_len,
tvb, offset, 2, sh_alg_length);
offset += 2;
if (ext_len < 2 || sh_alg_length != ext_len - 2) {
/* ERROR: sh_alg_length must be 2 less than ext_len */
return offset;
}
ret = ssl_dissect_hash_alg_list(hf, tvb, tree, pinfo, offset, sh_alg_length);
if (ret >= 0)
offset += ret;
return offset;
} }
static gint static gint
@ -7082,9 +7074,9 @@ ssl_dissect_hnd_cert(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tree,
} }
void void
ssl_dissect_hnd_cert_req(ssl_common_dissect_t *hf, tvbuff_t *tvb, ssl_dissect_hnd_cert_req(ssl_common_dissect_t *hf, tvbuff_t *tvb, packet_info *pinfo,
proto_tree *tree, guint32 offset, packet_info *pinfo, proto_tree *tree, guint32 offset, guint32 offset_end,
const SslSession *session) const SslSession *session)
{ {
/* /*
* enum { * enum {
@ -7154,10 +7146,8 @@ ssl_dissect_hnd_cert_req(ssl_common_dissect_t *hf, tvbuff_t *tvb,
proto_item *ti; proto_item *ti;
proto_tree *subtree; proto_tree *subtree;
guint8 cert_types_count; guint8 cert_types_count;
gint sh_alg_length;
gint dnames_length; gint dnames_length;
asn1_ctx_t asn1_ctx; asn1_ctx_t asn1_ctx;
gint ret;
if (!tree) if (!tree)
return; return;
@ -7202,22 +7192,7 @@ ssl_dissect_hnd_cert_req(ssl_common_dissect_t *hf, tvbuff_t *tvb,
case TLSV1DOT2_VERSION: case TLSV1DOT2_VERSION:
case DTLSV1DOT2_VERSION: case DTLSV1DOT2_VERSION:
case TLSV1DOT3_VERSION: /* XXX draft -18 only, remove for next version */ case TLSV1DOT3_VERSION: /* XXX draft -18 only, remove for next version */
sh_alg_length = tvb_get_ntohs(tvb, offset); offset = ssl_dissect_hash_alg_list(hf, tvb, tree, pinfo, offset, offset_end);
if (sh_alg_length % 2) {
expert_add_info_format(pinfo, NULL,
&hf->ei.hs_sig_hash_alg_len_bad,
"Signature Hash Algorithm length (%d) must be a multiple of 2",
sh_alg_length);
return;
}
proto_tree_add_uint(tree, hf->hf.hs_sig_hash_alg_len,
tvb, offset, 2, sh_alg_length);
offset += 2;
ret = ssl_dissect_hash_alg_list(hf, tvb, tree, pinfo, offset, sh_alg_length);
if (ret >= 0)
offset += ret;
break; break;
default: default:

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

@ -818,9 +818,7 @@ typedef struct ssl_common_dissect {
expert_field malformed_trailing_data; expert_field malformed_trailing_data;
expert_field hs_ext_cert_status_undecoded; expert_field hs_ext_cert_status_undecoded;
expert_field hs_sig_hash_alg_len_bad;
expert_field hs_cipher_suites_len_bad; expert_field hs_cipher_suites_len_bad;
expert_field hs_sig_hash_algs_bad;
expert_field resumed; expert_field resumed;
expert_field record_length_invalid; expert_field record_length_invalid;
@ -922,9 +920,9 @@ ssl_dissect_hnd_cert(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tree,
GHashTable *key_hash, gint is_from_server); GHashTable *key_hash, gint is_from_server);
extern void extern void
ssl_dissect_hnd_cert_req(ssl_common_dissect_t *hf, tvbuff_t *tvb, ssl_dissect_hnd_cert_req(ssl_common_dissect_t *hf, tvbuff_t *tvb, packet_info *pinfo,
proto_tree *tree, guint32 offset, packet_info *pinfo, proto_tree *tree, guint32 offset, guint32 offset_end,
const SslSession *session); const SslSession *session);
extern void extern void
ssl_dissect_hnd_cli_cert_verify(ssl_common_dissect_t *hf, tvbuff_t *tvb, ssl_dissect_hnd_cli_cert_verify(ssl_common_dissect_t *hf, tvbuff_t *tvb,
@ -968,7 +966,6 @@ ssl_common_dissect_t name = { \
}, \ }, \
/* ei */ { \ /* ei */ { \
EI_INIT, EI_INIT, EI_INIT, EI_INIT, EI_INIT, EI_INIT, EI_INIT, \ EI_INIT, EI_INIT, EI_INIT, EI_INIT, EI_INIT, EI_INIT, EI_INIT, \
EI_INIT, EI_INIT, \
}, \ }, \
} }
/* }}} */ /* }}} */
@ -1628,18 +1625,10 @@ ssl_common_dissect_t name = { \
{ prefix ".handshake.status_request.undecoded", PI_UNDECODED, PI_NOTE, \ { prefix ".handshake.status_request.undecoded", PI_UNDECODED, PI_NOTE, \
"Responder ID list or Request Extensions are not implemented, contact Wireshark developers if you want this to be supported", EXPFILL } \ "Responder ID list or Request Extensions are not implemented, contact Wireshark developers if you want this to be supported", EXPFILL } \
}, \ }, \
{ & name .ei.hs_sig_hash_alg_len_bad, \
{ prefix ".handshake.sig_hash_alg_len.mult2", PI_MALFORMED, PI_ERROR, \
"Signature Hash Algorithm length must be a multiple of 2", EXPFILL } \
}, \
{ & name .ei.hs_cipher_suites_len_bad, \ { & name .ei.hs_cipher_suites_len_bad, \
{ prefix ".handshake.cipher_suites_length.mult2", PI_MALFORMED, PI_ERROR, \ { prefix ".handshake.cipher_suites_length.mult2", PI_MALFORMED, PI_ERROR, \
"Cipher suite length must be a multiple of 2", EXPFILL } \ "Cipher suite length must be a multiple of 2", EXPFILL } \
}, \ }, \
{ & name .ei.hs_sig_hash_algs_bad, \
{ prefix ".handshake.sig_hash_algs.mult2", PI_MALFORMED, PI_ERROR, \
"Hash Algorithm length must be a multiple of 2", EXPFILL } \
}, \
{ & name .ei.resumed, \ { & name .ei.resumed, \
{ prefix ".resumed", PI_SEQUENCE, PI_NOTE, \ { prefix ".resumed", PI_SEQUENCE, PI_NOTE, \
"This session reuses previously negotiated keys (Session resumption)", EXPFILL } \ "This session reuses previously negotiated keys (Session resumption)", EXPFILL } \

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

@ -2115,7 +2115,7 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
break; break;
case SSL_HND_CERT_REQUEST: case SSL_HND_CERT_REQUEST:
ssl_dissect_hnd_cert_req(&dissect_ssl3_hf, tvb, ssl_hand_tree, offset, pinfo, session); ssl_dissect_hnd_cert_req(&dissect_ssl3_hf, tvb, pinfo, ssl_hand_tree, offset, offset + length, session);
break; break;
case SSL_HND_SVR_HELLO_DONE: case SSL_HND_SVR_HELLO_DONE: