diff options
author | Peter Wu <peter@lekensteyn.nl> | 2017-01-27 22:30:34 +0100 |
---|---|---|
committer | Peter Wu <peter@lekensteyn.nl> | 2017-01-29 23:31:29 +0000 |
commit | b3035df88796e0e9058b315791861b03b3b59af7 (patch) | |
tree | 3bd634d05efcbf85297c658fda8741a074c75ea0 /epan | |
parent | 9fb9bc52bbed0b8be199e0f3ea21c6e543cde2ab (diff) |
(D)TLS: fix type of record sequence number
The record sequence number is 64-bit, not 32-bit. This applies to all
SSLv3/TLS/DTLS versions. Without this fix, after about four million
records, the wrong MAC is calculated (for TLS 1.2) or decryption will
fail (for TLS 1.3).
Change-Id: I05e5e8bc4229ac443a1b06c5fe984fb885eab1ca
Reviewed-on: https://code.wireshark.org/review/19824
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Diffstat (limited to 'epan')
-rw-r--r-- | epan/dissectors/packet-dtls.c | 4 | ||||
-rw-r--r-- | epan/dissectors/packet-ssl-utils.c | 24 | ||||
-rw-r--r-- | epan/dissectors/packet-ssl-utils.h | 2 | ||||
-rw-r--r-- | epan/dissectors/packet-ssl.c | 5 |
4 files changed, 12 insertions, 23 deletions
diff --git a/epan/dissectors/packet-dtls.c b/epan/dissectors/packet-dtls.c index 9ac75a4d3f..a13bbf9bc9 100644 --- a/epan/dissectors/packet-dtls.c +++ b/epan/dissectors/packet-dtls.c @@ -714,13 +714,13 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo, if(ssl){ if(ssl_packet_from_server(session, dtls_associations, pinfo)){ if (ssl->server) { - ssl->server->seq=(guint32)sequence_number; + ssl->server->seq=sequence_number; ssl->server->epoch=epoch; } } else{ if (ssl->client) { - ssl->client->seq=(guint32)sequence_number; + ssl->client->seq=sequence_number; ssl->client->epoch=epoch; } } diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c index d55757c88d..76ff8f0865 100644 --- a/epan/dissectors/packet-ssl-utils.c +++ b/epan/dissectors/packet-ssl-utils.c @@ -3329,7 +3329,7 @@ create_decoders: ssl_session->client_new->flow = ssl_session->client ? ssl_session->client->flow : ssl_create_flow(); ssl_session->server_new->flow = ssl_session->server ? ssl_session->server->flow : ssl_create_flow(); - ssl_debug_printf("%s: client seq %d, server seq %d\n", + ssl_debug_printf("%s: client seq %" G_GUINT64_FORMAT ", server seq %" G_GUINT64_FORMAT "\n", G_STRFUNC, ssl_session->client_new->seq, ssl_session->server_new->seq); g_free(key_block.data); ssl_session->state |= SSL_HAVE_SESSION_KEY; @@ -3393,18 +3393,6 @@ ssl_decrypt_pre_master_secret(SslDecryptSession*ssl_session, #endif /* HAVE_LIBGNUTLS */ /* Decryption integrity check {{{ */ -/* convert network byte order 32 byte number to right-aligned host byte order * - * 8 bytes buffer */ -static gint fmt_seq(guint32 num, guint8* buf) -{ - guint32 netnum; - - memset(buf,0,8); - netnum=g_htonl(num); - memcpy(buf+4,&netnum,4); - - return(0); -} static gint tls_check_mac(SslDecoder*decoder, gint ct, gint ver, guint8* data, @@ -3424,7 +3412,7 @@ tls_check_mac(SslDecoder*decoder, gint ct, gint ver, guint8* data, return -1; /* hash sequence number */ - fmt_seq(decoder->seq,buf); + phton64(buf, decoder->seq); decoder->seq++; @@ -3483,7 +3471,7 @@ ssl3_check_mac(SslDecoder*decoder,int ct,guint8* data, ssl_md_update(&mc,buf,pad_ct); /* hash sequence number */ - fmt_seq(decoder->seq,buf); + phton64(buf, decoder->seq); decoder->seq++; ssl_md_update(&mc,buf,8); @@ -3537,9 +3525,9 @@ dtls_check_mac(SslDecoder*decoder, gint ct,int ver, guint8* data, if (ssl_hmac_init(&hm,decoder->mac_key.data,decoder->mac_key.data_len,md) != 0) return -1; - ssl_debug_printf("dtls_check_mac seq: %d epoch: %d\n",decoder->seq,decoder->epoch); + ssl_debug_printf("dtls_check_mac seq: %" G_GUINT64_FORMAT " epoch: %d\n",decoder->seq,decoder->epoch); /* hash sequence number */ - fmt_seq(decoder->seq,buf); + phton64(buf, decoder->seq); buf[0]=decoder->epoch>>8; buf[1]=(guint8)decoder->epoch; @@ -3743,7 +3731,7 @@ ssl_decrypt_record(SslDecryptSession*ssl,SslDecoder* decoder, gint ct, } /* Now check the MAC */ - ssl_debug_printf("checking mac (len %d, version %X, ct %d seq %d)\n", + ssl_debug_printf("checking mac (len %d, version %X, ct %d seq %" G_GUINT64_FORMAT ")\n", worklen, ssl->session.version, ct, decoder->seq); if(ssl->session.version==SSLV3_VERSION){ if(ssl3_check_mac(decoder,ct,out_str->data,worklen,mac) < 0) { diff --git a/epan/dissectors/packet-ssl-utils.h b/epan/dissectors/packet-ssl-utils.h index 39875c6191..8760103922 100644 --- a/epan/dissectors/packet-ssl-utils.h +++ b/epan/dissectors/packet-ssl-utils.h @@ -302,7 +302,7 @@ typedef struct _SslDecoder { StringInfo write_iv; /* for AEAD ciphers (at least GCM, CCM) */ SSL_CIPHER_CTX evp; SslDecompress *decomp; - guint32 seq; + guint64 seq; /**< Implicit (TLS) or explicit (DTLS) record sequence number. */ guint16 epoch; SslFlow *flow; } SslDecoder; diff --git a/epan/dissectors/packet-ssl.c b/epan/dissectors/packet-ssl.c index 02413fa409..acd30d2f47 100644 --- a/epan/dissectors/packet-ssl.c +++ b/epan/dissectors/packet-ssl.c @@ -3350,13 +3350,14 @@ void ssl_set_master_secret(guint32 frame_num, address *addr_srv, address *addr_c ssl_change_cipher(ssl, FALSE); /* update seq numbers if available */ + /* TODO change API to accept 64-bit sequence numbers. */ if (ssl->client && (client_seq != (guint32)-1)) { ssl->client->seq = client_seq; - ssl_debug_printf("ssl_set_master_secret client->seq updated to %u\n", ssl->client->seq); + ssl_debug_printf("ssl_set_master_secret client->seq updated to %" G_GUINT64_FORMAT "\n", ssl->client->seq); } if (ssl->server && (server_seq != (guint32)-1)) { ssl->server->seq = server_seq; - ssl_debug_printf("ssl_set_master_secret server->seq updated to %u\n", ssl->server->seq); + ssl_debug_printf("ssl_set_master_secret server->seq updated to %" G_GUINT64_FORMAT "\n", ssl->server->seq); } /* update IV from last data */ |