(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>

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;
      }
     }

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) {

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;

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 */

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))