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

ssl: add missing valid handshake types 

The DTLS dissector duplicated a handshake types check, this has been
eliminated. Convert HandshakeType and ContentType to enums to get the
benefit of compiler-checked switch cases. Move these checks to
ssl-utils.

Two default cases could never be reached since the dissector returns
immediately on an invalid ContentType.

Also fixed misleading debugging messages.

Change-Id: I07a2062564e073004dcc0401cd82538e5659fa0c
Reviewed-on: https://code.wireshark.org/review/2978
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
This commit is contained in:
Peter Wu 2014-07-10 11:50:23 +02:00 committed by Anders Broman
parent c7b45d0a7d
commit 7248c24afc
4 changed files with 98 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -380,7 +380,6 @@ static void dissect_dtls_hnd_finished(tvbuff_t *tvb,
*
*/
/*static void ssl_set_conv_version(packet_info *pinfo, guint version);*/
static gint dtls_is_valid_handshake_type(guint8 type);
static gint dtls_is_authoritative_version_message(guint8 content_type,
guint8 next_byte);
@ -878,7 +877,7 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo,
/* PAOLO try to decrypt each record (we must keep ciphers "in sync")
* store plain text only for app data */
switch (content_type) {
switch ((ContentType) content_type) {
case SSL_ID_CHG_CIPHER_SPEC:
col_append_str(pinfo->cinfo, COL_INFO, "Change Cipher Spec");
dissect_dtls_change_cipher_spec(tvb, dtls_record_tree,
@ -1005,7 +1004,7 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo,
offset, record_length, ENC_NA);
break;
case SSL_ID_HEARTBEAT:
{
{
tvbuff_t* decrypted;
if (ssl && decrypt_dtls_record(tvb, pinfo, offset,
@ -1024,12 +1023,7 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo,
session, record_length, FALSE);
}
break;
}
default:
/* shouldn't get here since we check above for valid types */
col_append_str(pinfo->cinfo, COL_INFO, "Bad DTLS Content Type");
break;
}
}
offset += record_length; /* skip to end of record */
@ -1169,7 +1163,6 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo,
guint32 fragment_offset;
guint32 fragment_length;
gboolean first_iteration;
gboolean frag_hand;
guint32 reassembled_length;
msg_type_str = NULL;
@ -1278,29 +1271,8 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo,
{
fragmented = TRUE;
/* Handle fragments of known message type */
switch (msg_type) {
case SSL_HND_HELLO_REQUEST:
case SSL_HND_CLIENT_HELLO:
case SSL_HND_HELLO_VERIFY_REQUEST:
case SSL_HND_NEWSESSION_TICKET:
case SSL_HND_SERVER_HELLO:
case SSL_HND_CERTIFICATE:
case SSL_HND_SERVER_KEY_EXCHG:
case SSL_HND_CERT_REQUEST:
case SSL_HND_SVR_HELLO_DONE:
case SSL_HND_CERT_VERIFY:
case SSL_HND_CLIENT_KEY_EXCHG:
case SSL_HND_FINISHED:
frag_hand = TRUE;
break;
default:
/* Ignore encrypted handshake messages */
frag_hand = FALSE;
break;
}
if (frag_hand)
/* Handle fragments of known message type, ignore others */
if (ssl_is_valid_handshake_type(msg_type, TRUE))
{
/* Fragmented handshake message */
pinfo->fragmented = TRUE;
@ -1406,7 +1378,7 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo,
}
/* now dissect the handshake message, if necessary */
switch (msg_type) {
switch ((HandshakeType) msg_type) {
case SSL_HND_HELLO_REQUEST:
/* hello_request has no fields, so nothing to do! */
break;
@ -1469,6 +1441,13 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo,
case SSL_HND_FINISHED:
dissect_dtls_hnd_finished(sub_tvb, ssl_hand_tree, 0, session);
break;
case SSL_HND_CERT_URL:
case SSL_HND_CERT_STATUS:
case SSL_HND_SUPPLEMENTAL_DATA:
case SSL_HND_ENCRYPTED_EXTS:
/* TODO: does this need further dissection? */
break;
}
}
@ -2192,33 +2171,11 @@ ssl_set_conv_version(packet_info *pinfo, guint version)
}
#endif
static gint
dtls_is_valid_handshake_type(guint8 type)
{
switch (type) {
case SSL_HND_HELLO_REQUEST:
case SSL_HND_CLIENT_HELLO:
case SSL_HND_SERVER_HELLO:
case SSL_HND_HELLO_VERIFY_REQUEST:
case SSL_HND_NEWSESSION_TICKET:
case SSL_HND_CERTIFICATE:
case SSL_HND_SERVER_KEY_EXCHG:
case SSL_HND_CERT_REQUEST:
case SSL_HND_SVR_HELLO_DONE:
case SSL_HND_CERT_VERIFY:
case SSL_HND_CLIENT_KEY_EXCHG:
case SSL_HND_FINISHED:
return 1;
}
return 0;
}
static gint
dtls_is_authoritative_version_message(guint8 content_type, guint8 next_byte)
{
if (content_type == SSL_ID_HANDSHAKE
&& dtls_is_valid_handshake_type(next_byte))
&& ssl_is_valid_handshake_type(next_byte, TRUE))
{
return (next_byte != SSL_HND_CLIENT_HELLO);
}

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

@ -4368,15 +4368,46 @@ ssl_restore_session_ticket(SslDecryptSession* ssl, GHashTable *session_hash)
return TRUE;
}
int
gboolean
ssl_is_valid_content_type(guint8 type)
{
if ((type >= 0x14) && (type <= 0x18))
{
return 1;
switch ((ContentType) type) {
case SSL_ID_CHG_CIPHER_SPEC:
case SSL_ID_ALERT:
case SSL_ID_HANDSHAKE:
case SSL_ID_APP_DATA:
case SSL_ID_HEARTBEAT:
return TRUE;
}
return FALSE;
}
return 0;
gboolean
ssl_is_valid_handshake_type(guint8 hs_type, gboolean is_dtls)
{
switch ((HandshakeType) hs_type) {
case SSL_HND_HELLO_VERIFY_REQUEST:
/* hello_verify_request is DTLS-only */
return is_dtls;
case SSL_HND_HELLO_REQUEST:
case SSL_HND_CLIENT_HELLO:
case SSL_HND_SERVER_HELLO:
case SSL_HND_NEWSESSION_TICKET:
case SSL_HND_CERTIFICATE:
case SSL_HND_SERVER_KEY_EXCHG:
case SSL_HND_CERT_REQUEST:
case SSL_HND_SVR_HELLO_DONE:
case SSL_HND_CERT_VERIFY:
case SSL_HND_CLIENT_KEY_EXCHG:
case SSL_HND_FINISHED:
case SSL_HND_CERT_URL:
case SSL_HND_CERT_STATUS:
case SSL_HND_SUPPLEMENTAL_DATA:
case SSL_HND_ENCRYPTED_EXTS:
return TRUE;
}
return FALSE;
}
static const unsigned int kRSAMasterSecretLength = 48; /* RFC5246 8.1 */
@ -4426,7 +4457,7 @@ ssl_keylog_parse_session_id(const char* line,
return FALSE;
ssl_session->state &= ~(SSL_PRE_MASTER_SECRET|SSL_HAVE_SESSION_KEY);
ssl_session->state |= SSL_MASTER_SECRET;
ssl_debug_printf("found master secret in key log\n");
ssl_debug_printf("found master secret in key log via RSA Session-ID\n");
return TRUE;
}
@ -4473,7 +4504,7 @@ ssl_keylog_parse_client_random(const char* line,
return FALSE;
ssl_session->state &= ~(SSL_PRE_MASTER_SECRET|SSL_HAVE_SESSION_KEY);
ssl_session->state |= SSL_MASTER_SECRET;
ssl_debug_printf("found master secret in key log\n");
ssl_debug_printf("found master secret in key log via CLIENT_RANDOM\n");
return TRUE;
}

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

@ -72,30 +72,34 @@
#define SSL_VER_TLSv1DOT2 7
/* other defines */
#define SSL_ID_CHG_CIPHER_SPEC 0x14
#define SSL_ID_ALERT 0x15
#define SSL_ID_HANDSHAKE 0x16
#define SSL_ID_APP_DATA 0x17
#define SSL_ID_HEARTBEAT 0x18
typedef enum {
SSL_ID_CHG_CIPHER_SPEC = 0x14,
SSL_ID_ALERT = 0x15,
SSL_ID_HANDSHAKE = 0x16,
SSL_ID_APP_DATA = 0x17,
SSL_ID_HEARTBEAT = 0x18
} ContentType;
#define SSL_HND_HELLO_REQUEST 0
#define SSL_HND_CLIENT_HELLO 1
#define SSL_HND_SERVER_HELLO 2
#define SSL_HND_HELLO_VERIFY_REQUEST 3
#define SSL_HND_NEWSESSION_TICKET 4
#define SSL_HND_CERTIFICATE 11
#define SSL_HND_SERVER_KEY_EXCHG 12
#define SSL_HND_CERT_REQUEST 13
#define SSL_HND_SVR_HELLO_DONE 14
#define SSL_HND_CERT_VERIFY 15
#define SSL_HND_CLIENT_KEY_EXCHG 16
#define SSL_HND_FINISHED 20
#define SSL_HND_CERT_URL 21
#define SSL_HND_CERT_STATUS 22
#define SSL_HND_SUPPLEMENTAL_DATA 23
/* Encrypted Extensions was NextProtocol in draft-agl-tls-nextprotoneg-03 and
* changed in draft 04 */
#define SSL_HND_ENCRYPTED_EXTS 67
typedef enum {
SSL_HND_HELLO_REQUEST = 0,
SSL_HND_CLIENT_HELLO = 1,
SSL_HND_SERVER_HELLO = 2,
SSL_HND_HELLO_VERIFY_REQUEST = 3,
SSL_HND_NEWSESSION_TICKET = 4,
SSL_HND_CERTIFICATE = 11,
SSL_HND_SERVER_KEY_EXCHG = 12,
SSL_HND_CERT_REQUEST = 13,
SSL_HND_SVR_HELLO_DONE = 14,
SSL_HND_CERT_VERIFY = 15,
SSL_HND_CLIENT_KEY_EXCHG = 16,
SSL_HND_FINISHED = 20,
SSL_HND_CERT_URL = 21,
SSL_HND_CERT_STATUS = 22,
SSL_HND_SUPPLEMENTAL_DATA = 23,
/* Encrypted Extensions was NextProtocol in draft-agl-tls-nextprotoneg-03
* and changed in draft 04 */
SSL_HND_ENCRYPTED_EXTS = 67
} HandshakeType;
#define SSL2_HND_ERROR 0x00
#define SSL2_HND_CLIENT_HELLO 0x01
@ -348,6 +352,7 @@ typedef struct _SslSession {
gint8 server_cert_type;
} SslSession;
/* This holds state information for a SSL conversation */
typedef struct _SslDecryptSession {
guchar _master_secret[48];
guchar _session_id[256];
@ -590,9 +595,12 @@ ssl_save_session_ticket(SslDecryptSession* ssl, GHashTable *session_hash);
extern gboolean
ssl_restore_session_ticket(SslDecryptSession* ssl, GHashTable *session_hash);
extern gint
extern gboolean
ssl_is_valid_content_type(guint8 type);
extern gboolean
ssl_is_valid_handshake_type(guint8 hs_type, gboolean is_dtls);
/* common header fields, subtrees and expert info for SSL and DTLS dissectors */
typedef struct ssl_common_dissect {
struct {

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

@ -590,7 +590,6 @@ static void dissect_pct_msg_error(tvbuff_t *tvb,
*
*/
/*static void ssl_set_conv_version(packet_info *pinfo, guint version);*/
static gint ssl_is_valid_handshake_type(const guint8 type);
static gint ssl_is_valid_ssl_version(const guint16 version);
static gint ssl_is_authoritative_version_message(const guint8 content_type,
const guint8 next_byte);
@ -1619,7 +1618,7 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
/* PAOLO try to decrypt each record (we must keep ciphers "in sync")
* store plain text only for app data */
switch (content_type) {
switch ((ContentType) content_type) {
case SSL_ID_CHG_CIPHER_SPEC:
ssl_debug_printf("dissect_ssl3_change_cipher_spec\n");
col_append_str(pinfo->cinfo, COL_INFO, "Change Cipher Spec");
@ -1707,7 +1706,7 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
break;
case SSL_ID_HEARTBEAT:
{
{
tvbuff_t *decrypted;
if (ssl && decrypt_ssl3_record(tvb, pinfo, offset,
@ -1733,12 +1732,7 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
dissect_ssl3_heartbeat(tvb, pinfo, ssl_record_tree, offset, session, record_length, plaintext);
}
break;
}
default:
/* shouldn't get here since we check above for valid types */
col_append_str(pinfo->cinfo, COL_INFO, "Bad SSLv3 Content Type");
break;
}
}
offset += record_length; /* skip to end of record */
@ -1980,7 +1974,7 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
offset += 3;
/* now dissect the handshake message, if necessary */
switch (msg_type) {
switch ((HandshakeType) msg_type) {
case SSL_HND_HELLO_REQUEST:
/* hello_request has no fields, so nothing to do! */
break;
@ -1993,6 +1987,10 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
dissect_ssl3_hnd_srv_hello(tvb, ssl_hand_tree, offset, length, session, ssl);
break;
case SSL_HND_HELLO_VERIFY_REQUEST:
/* only valid for DTLS */
break;
case SSL_HND_NEWSESSION_TICKET:
dissect_ssl3_hnd_new_ses_ticket(tvb, ssl_hand_tree, offset, length, ssl);
break;
@ -2051,6 +2049,10 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
dissect_ssl3_hnd_cert_status(tvb, ssl_hand_tree, offset, pinfo);
break;
case SSL_HND_SUPPLEMENTAL_DATA:
/* TODO: dissect this? */
break;
case SSL_HND_ENCRYPTED_EXTS:
dissect_ssl3_hnd_encrypted_exts(tvb, ssl_hand_tree, offset);
break;
@ -2193,7 +2195,7 @@ dissect_ssl3_hnd_hello_common(tvbuff_t *tvb, proto_tree *tree,
if (!ssl_restore_session(ssl, ssl_session_hash)) {
/* If we failed to find the previous session, we may still have
* the master secret in the key log. */
if (ssl_keylog_lookup(ssl, ssl_options.keylog_filename, NULL)) {
if (!ssl_keylog_lookup(ssl, ssl_options.keylog_filename, NULL)) {
ssl_debug_printf(" cannot find master secret in keylog file either\n");
} else {
ssl_debug_printf(" found master secret in keylog file\n");
@ -4027,27 +4029,6 @@ ssl_set_conv_version(packet_info *pinfo, guint version)
}
#endif
static gint
ssl_is_valid_handshake_type(const guint8 type)
{
switch (type) {
case SSL_HND_HELLO_REQUEST:
case SSL_HND_CLIENT_HELLO:
case SSL_HND_SERVER_HELLO:
case SSL_HND_NEWSESSION_TICKET:
case SSL_HND_CERTIFICATE:
case SSL_HND_SERVER_KEY_EXCHG:
case SSL_HND_CERT_REQUEST:
case SSL_HND_SVR_HELLO_DONE:
case SSL_HND_CERT_VERIFY:
case SSL_HND_CLIENT_KEY_EXCHG:
case SSL_HND_FINISHED:
return 1;
}
return 0;
}
static gint
ssl_is_valid_ssl_version(const guint16 version)
{
@ -4062,7 +4043,7 @@ ssl_is_authoritative_version_message(const guint8 content_type,
const guint8 next_byte)
{
if (content_type == SSL_ID_HANDSHAKE
&& ssl_is_valid_handshake_type(next_byte))
&& ssl_is_valid_handshake_type(next_byte, FALSE))
{
return (next_byte != SSL_HND_CLIENT_HELLO);
}