From ec9ce3fdad014274ce00de1768f9e11395a77e37 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Fri, 27 Jan 2017 22:30:34 +0100 Subject: (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 --- epan/dissectors/packet-dtls.c | 4 ++-- epan/dissectors/packet-ssl-utils.c | 24 ++++++------------------ epan/dissectors/packet-ssl-utils.h | 2 +- epan/dissectors/packet-ssl.c | 5 +++-- wsutil/pint.h | 11 +++++++++++ 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/epan/dissectors/packet-dtls.c b/epan/dissectors/packet-dtls.c index dd08f58da4..0ac6a94340 100644 --- a/epan/dissectors/packet-dtls.c +++ b/epan/dissectors/packet-dtls.c @@ -716,13 +716,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 dbe56d89ee..ee016e389f 100644 --- a/epan/dissectors/packet-ssl.c +++ b/epan/dissectors/packet-ssl.c @@ -3353,13 +3353,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 */ diff --git a/wsutil/pint.h b/wsutil/pint.h index 02012537ee..ecea5fc94a 100644 --- a/wsutil/pint.h +++ b/wsutil/pint.h @@ -137,6 +137,17 @@ ((guint8*)(p))[3] = (guint8)((v) >> 0); \ } +static inline void phton64(guint8 *p, guint64 v) { + p[0] = (guint8)(v >> 56); + p[1] = (guint8)(v >> 48); + p[2] = (guint8)(v >> 40); + p[3] = (guint8)(v >> 32); + p[4] = (guint8)(v >> 24); + p[5] = (guint8)(v >> 16); + p[6] = (guint8)(v >> 8); + p[7] = (guint8)(v >> 0); +} + /* Subtract two guint32s with respect to wraparound */ #define guint32_wraparound_diff(higher, lower) ((higher>lower)?(higher-lower):(higher+0xffffffff-lower+1)) -- cgit v1.2.1