ssl-utils: refactor AEAD decryption handling

The current ssl_decrypt_record is hard to understand due to mixing CBC
concepts (MAC, padding) with AEAD. Extract the AEAD functionality and
use better variable naming.

The "Plaintext" debug print now includes just the plaintext (the auth
tag is stripped). A write_iv.data_len check is added just to be sure and
more prep work is done for auth tag validation and TLS 1.3 support.

Tested against the (D)TLS AEAD tests on Libgcrypt 1.4.5 (CentOS 6),
1.6.5 (Ubuntu 14.04), 1.7.6 (Arch Linux). Compile-tested w/o Libgcrypt.

Change-Id: I94dd2fd70e1281d85c954abfe523f7483d9ac68b
Reviewed-on: https://code.wireshark.org/review/19852
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Anders Broman <a.broman58@gmail.com>

4 files changed, 137 insertions, 95 deletions

diff --git a/epan/dissectors/packet-dtls.c b/epan/dissectors/packet-dtls.c
index e85c2b6d86..a90c8e0432 100644
--- a/epan/dissectors/packet-dtls.c
+++ b/epan/dissectors/packet-dtls.c
@@ -566,8 +566,8 @@ dtls_is_null_cipher(guint cipher )
 }
 
 static gboolean
-decrypt_dtls_record(tvbuff_t *tvb, packet_info *pinfo, guint32 offset,
-                    guint32 record_length, guint8 content_type, SslDecryptSession* ssl,
+decrypt_dtls_record(tvbuff_t *tvb, packet_info *pinfo, guint32 offset, SslDecryptSession *ssl,
+                    guint8 content_type, guint16 record_version, guint16 record_length,
                     gboolean allow_fragments)
 {
   gboolean    success;
@@ -616,7 +616,8 @@ decrypt_dtls_record(tvbuff_t *tvb, packet_info *pinfo, guint32 offset,
       ssl_debug_printf("decrypt_dtls_record: no decoder available\n");
       return FALSE;
     }
-    success = ssl_decrypt_record(ssl, decoder, content_type, tvb_get_ptr(tvb, offset, record_length), record_length,
+    success = ssl_decrypt_record(ssl, decoder, content_type, record_version,
+                           tvb_get_ptr(tvb, offset, record_length), record_length,
                            &dtls_compressed_data, &dtls_decrypted_data, &dtls_decrypted_data_avail) == 0;
   }
   else if (dtls_is_null_cipher(ssl->session.cipher)) {
@@ -792,7 +793,7 @@ dissect_dtls_record(tvbuff_t *tvb, packet_info *pinfo,
   /* try to decrypt record on the first pass, if possible. Store decrypted
    * record for later usage (without having to decrypt again). */
   if (ssl) {
-    decrypt_dtls_record(tvb, pinfo, offset, record_length, content_type, ssl,
+    decrypt_dtls_record(tvb, pinfo, offset, ssl, content_type, version, record_length,
         content_type == SSL_ID_APP_DATA || content_type == SSL_ID_HANDSHAKE);
   }
   decrypted = ssl_get_record_info(tvb, proto_dtls, pinfo, tvb_raw_offset(tvb)+offset, &record);
diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c
index 76ff8f0865..5bbd07f979 100644
--- a/epan/dissectors/packet-ssl-utils.c
+++ b/epan/dissectors/packet-ssl-utils.c
@@ -2686,12 +2686,12 @@ ssl_change_cipher(SslDecryptSession *ssl_session, gboolean server)
 }
 
 int
-ssl_decrypt_record(SslDecryptSession*ssl, SslDecoder* decoder, gint ct,
-        const guchar* in, guint inl, StringInfo* comp_str _U_, StringInfo* out, guint* outl)
+ssl_decrypt_record(SslDecryptSession *ssl, SslDecoder *decoder, guint8 ct, guint16 record_version,
+        const guchar *in, guint16 inl, StringInfo *comp_str _U_, StringInfo *out_str, guint *outl)
 {
     ssl_debug_printf("ssl_decrypt_record: impossible without gnutls. ssl %p"
-        "decoder %p ct %d, in %p inl %d out %p outl %p\n", ssl, decoder, ct,
-        in, inl, out, outl);
+        "decoder %p ct %d version %d in %p inl %d out %p outl %p\n", ssl, decoder, ct,
+        record_version, in, inl, out_str, outl);
     return 0;
 }
 /* }}} */
@@ -3558,12 +3558,102 @@ dtls_check_mac(SslDecoder*decoder, gint ct,int ver, guint8* data,
 }
 /* Decryption integrity check }}} */
 
+
+static gboolean
+tls_decrypt_aead_record(SslDecryptSession *ssl, SslDecoder *decoder,
+        guint8 ct _U_, guint16 record_version _U_,
+        const guchar *in, guint16 inl, StringInfo *out_str, guint *outl)
+{
+    /* RFC 5246 (TLS 1.2) 6.2.3.3 defines the TLSCipherText.fragment as:
+     * GenericAEADCipher: { nonce_explicit, [content] }
+     * In TLS 1.3 this explicit nonce is gone.
+     * With AES GCM/CCM, "[content]" is actually the concatenation of the
+     * ciphertext and authentication tag.
+     */
+    const guint16   version = ssl->session.version;
+    const gboolean  is_v12 = version == TLSV1DOT2_VERSION || version == DTLSV1DOT2_VERSION;
+    gcry_error_t    err;
+    const guchar   *explicit_nonce = NULL, *ciphertext;
+    guint           ciphertext_len, auth_tag_len;
+    guchar          nonce[12];
+    guchar          nonce_with_counter[16] = { 0 };
+
+    switch (decoder->cipher_suite->mode) {
+    case MODE_GCM:
+    case MODE_CCM:
+        auth_tag_len = 16;
+        break;
+    case MODE_CCM_8:
+        auth_tag_len = 8;
+        break;
+    default:
+        ssl_debug_printf("%s unsupported cipher!\n", G_STRFUNC);
+        return FALSE;
+    }
+
+    /* Parse input into explicit nonce (TLS 1.2 only), ciphertext and tag. */
+    if (is_v12) {
+        if (inl < EXPLICIT_NONCE_LEN + auth_tag_len) {
+            ssl_debug_printf("%s input %d is too small for explicit nonce %d and auth tag %d\n",
+                    G_STRFUNC, inl, EXPLICIT_NONCE_LEN, auth_tag_len);
+            return FALSE;
+        }
+        explicit_nonce = in;
+        ciphertext = explicit_nonce + EXPLICIT_NONCE_LEN;
+        ciphertext_len = inl - EXPLICIT_NONCE_LEN - auth_tag_len;
+    } else {
+        ssl_debug_printf("%s Unexpected TLS version %#x\n", G_STRFUNC, version);
+        return FALSE;
+    }
+
+    /* Nonce construction is version-specific. */
+    if (is_v12) {
+        DISSECTOR_ASSERT(decoder->write_iv.data_len == IMPLICIT_NONCE_LEN);
+        /* Implicit (4) and explicit (8) part of nonce. */
+        memcpy(nonce, decoder->write_iv.data, IMPLICIT_NONCE_LEN);
+        memcpy(nonce + IMPLICIT_NONCE_LEN, explicit_nonce, EXPLICIT_NONCE_LEN);
+
+        if (decoder->cipher_suite->mode == MODE_GCM) {
+            /* NIST SP 800-38D, sect. 7.2 says that the 32-bit counter part starts
+             * at 1, and gets incremented before passing to the block cipher. */
+            memcpy(nonce_with_counter, nonce, IMPLICIT_NONCE_LEN + EXPLICIT_NONCE_LEN);
+            nonce_with_counter[IMPLICIT_NONCE_LEN + EXPLICIT_NONCE_LEN + 3] = 2;
+        } else { /* MODE_CCM and MODE_CCM_8 */
+            /* The nonce for CCM and GCM are the same, but the nonce is used as input
+             * in the CCM algorithm described in RFC 3610. The nonce generated here is
+             * the one from RFC 3610 sect 2.3. Encryption. */
+            /* Flags: (L-1) ; L = 16 - 1 - nonceSize */
+            nonce_with_counter[0] = 3 - 1;
+            memcpy(nonce_with_counter + 1, nonce, IMPLICIT_NONCE_LEN + EXPLICIT_NONCE_LEN);
+            /* struct { opaque salt[4]; opaque nonce_explicit[8] } CCMNonce (RFC 6655) */
+            nonce_with_counter[IMPLICIT_NONCE_LEN + EXPLICIT_NONCE_LEN + 3] = 1;
+        }
+    }
+
+    err = gcry_cipher_setctr(decoder->evp, nonce_with_counter, 16);
+    if (err) {
+        ssl_debug_printf("%s failed: failed to set CTR: %s\n", G_STRFUNC, gcry_strerror(err));
+        return FALSE;
+    }
+
+    /* Decrypt now that nonce and AAD are set. */
+    err = gcry_cipher_decrypt(decoder->evp, out_str->data, out_str->data_len, ciphertext, ciphertext_len);
+    if (err) {
+        ssl_debug_printf("%s decrypt failed: %s\n", G_STRFUNC, gcry_strerror(err));
+        return FALSE;
+    }
+
+    ssl_print_data("Plaintext", out_str->data, ciphertext_len);
+    *outl = ciphertext_len;
+    return TRUE;
+}
+
 /* Record decryption glue based on security parameters {{{ */
 /* Assume that we are called only for a non-NULL decoder which also means that
  * we have a non-NULL decoder->cipher_suite. */
 int
-ssl_decrypt_record(SslDecryptSession*ssl,SslDecoder* decoder, gint ct,
-        const guchar* in, guint inl, StringInfo* comp_str, StringInfo* out_str, guint* outl)
+ssl_decrypt_record(SslDecryptSession *ssl, SslDecoder *decoder, guint8 ct, guint16 record_version,
+        const guchar *in, guint16 inl, StringInfo *comp_str, StringInfo *out_str, guint *outl)
 {
     guint   pad, worklen, uncomplen;
     guint8 *mac;
@@ -3579,6 +3669,21 @@ ssl_decrypt_record(SslDecryptSession*ssl,SslDecoder* decoder, gint ct,
         ssl_data_realloc(out_str, inl + 32);
     }
 
+    /* AEAD ciphers (GenericAEADCipher in TLS 1.2; TLS 1.3) have no padding nor
+     * a separate MAC, so use a different routine for simplicity. */
+    if (decoder->cipher_suite->mode == MODE_GCM ||
+        decoder->cipher_suite->mode == MODE_CCM ||
+        decoder->cipher_suite->mode == MODE_CCM_8 ||
+        ssl->session.version == TLSV1DOT3_VERSION) {
+
+        if (!tls_decrypt_aead_record(ssl, decoder, ct, record_version, in, inl, out_str, &worklen)) {
+            /* decryption failed */
+            return -1;
+        }
+
+        goto skip_mac;
+    }
+
     /* RFC 6101/2246: SSLCipherText/TLSCipherText has two structures for types:
      * (notation: { unencrypted, [ encrypted ] })
      * GenericStreamCipher: { [content, mac] }
@@ -3617,50 +3722,8 @@ ssl_decrypt_record(SslDecryptSession*ssl,SslDecoder* decoder, gint ct,
         }
     }
 
-    /* Nonce for GenericAEADCipher */
-    if (decoder->cipher_suite->mode == MODE_GCM ||
-        decoder->cipher_suite->mode == MODE_CCM ||
-        decoder->cipher_suite->mode == MODE_CCM_8) {
-        /* 4 bytes write_iv, 8 bytes explicit_nonce, 4 bytes counter */
-        guchar gcm_nonce[16] = { 0 };
-
-        if ((gint)inl < SSL_EX_NONCE_LEN_GCM) {
-            ssl_debug_printf("ssl_decrypt_record failed: input %d has no space for nonce %d\n",
-                inl, SSL_EX_NONCE_LEN_GCM);
-            return -1;
-        }
-
-        if (decoder->cipher_suite->mode == MODE_GCM) {
-            memcpy(gcm_nonce, decoder->write_iv.data, decoder->write_iv.data_len); /* salt */
-            memcpy(gcm_nonce + decoder->write_iv.data_len, in, SSL_EX_NONCE_LEN_GCM);
-            /* NIST SP 800-38D, sect. 7.2 says that the 32-bit counter part starts
-             * at 1, and gets incremented before passing to the block cipher. */
-            gcm_nonce[4 + SSL_EX_NONCE_LEN_GCM + 3] = 2;
-        } else { /* MODE_CCM and MODE_CCM_8 */
-            /* The nonce for CCM and GCM are the same, but the nonce is used as input
-             * in the CCM algorithm described in RFC 3610. The nonce generated here is
-             * the one from RFC 3610 sect 2.3. Encryption. */
-            /* Flags: (L-1) ; L = 16 - 1 - nonceSize */
-            gcm_nonce[0] = 3 - 1;
-
-            /* struct { opaque salt[4]; opaque nonce_explicit[8] } CCMNonce (RFC 6655) */
-            memcpy(gcm_nonce + 1, decoder->write_iv.data, decoder->write_iv.data_len); /* salt */
-            memcpy(gcm_nonce + 1 + decoder->write_iv.data_len, in, SSL_EX_NONCE_LEN_GCM);
-            gcm_nonce[4 + SSL_EX_NONCE_LEN_GCM + 3] = 1;
-        }
-
-        pad = gcry_cipher_setctr (decoder->evp, gcm_nonce, sizeof (gcm_nonce));
-        if (pad != 0) {
-            ssl_debug_printf("ssl_decrypt_record failed: failed to set CTR: %s %s\n",
-                    gcry_strsource (pad), gcry_strerror (pad));
-            return -1;
-        }
-        inl -= SSL_EX_NONCE_LEN_GCM;
-        in += SSL_EX_NONCE_LEN_GCM;
-    }
-
     /* First decrypt*/
-    if ((pad = ssl_cipher_decrypt(&decoder->evp, out_str->data, out_str->data_len, in, inl))!= 0) {
+    if ((pad = ssl_cipher_decrypt(&decoder->evp, out_str->data, out_str->data_len, in, inl)) != 0) {
         ssl_debug_printf("ssl_decrypt_record failed: ssl_cipher_decrypt: %s %s\n", gcry_strsource (pad),
                     gcry_strerror (pad));
         return -1;
@@ -3669,26 +3732,6 @@ ssl_decrypt_record(SslDecryptSession*ssl,SslDecoder* decoder, gint ct,
     ssl_print_data("Plaintext", out_str->data, inl);
     worklen=inl;
 
-    /* RFC 5116 sect 5.1/5.3: AES128/256 GCM/CCM uses 16 bytes for auth tag
-     * RFC 6655 sect 6.1: AEAD_AES_128_CCM uses 16 bytes for auth tag */
-    if (decoder->cipher_suite->mode == MODE_GCM ||
-        decoder->cipher_suite->mode == MODE_CCM) {
-        if (worklen < 16) {
-            ssl_debug_printf("ssl_decrypt_record failed: missing tag, work %d\n", worklen);
-            return -1;
-        }
-        /* XXX - validate auth tag */
-        worklen -= 16;
-    }
-    /* RFC 6655 sect 6.1: AEAD_AES_128_CCM_8 uses 8 bytes for auth tag */
-    if (decoder->cipher_suite->mode == MODE_CCM_8) {
-        if (worklen < 8) {
-            ssl_debug_printf("ssl_decrypt_record failed: missing tag, work %d\n", worklen);
-            return -1;
-        }
-        /* XXX - validate auth tag */
-        worklen -= 8;
-    }
 
     /* strip padding for GenericBlockCipher */
     if (decoder->cipher_suite->mode == MODE_CBC) {
@@ -3708,18 +3751,12 @@ ssl_decrypt_record(SslDecryptSession*ssl,SslDecoder* decoder, gint ct,
     }
 
     /* MAC for GenericStreamCipher and GenericBlockCipher */
-    if (decoder->cipher_suite->mode == MODE_STREAM ||
-        decoder->cipher_suite->mode == MODE_CBC) {
-        if (ssl_cipher_suite_dig(decoder->cipher_suite)->len > (gint)worklen) {
-            ssl_debug_printf("ssl_decrypt_record wrong record len/padding outlen %d\n work %d\n",*outl, worklen);
-            return -1;
-        }
-        worklen-=ssl_cipher_suite_dig(decoder->cipher_suite)->len;
-        mac = out_str->data + worklen;
-    } else /* if (decoder->cipher_suite->mode == MODE_GCM) */ {
-        /* GenericAEADCipher has no MAC */
-        goto skip_mac;
+    if (ssl_cipher_suite_dig(decoder->cipher_suite)->len > (gint)worklen) {
+        ssl_debug_printf("ssl_decrypt_record wrong record len/padding outlen %d\n work %d\n",*outl, worklen);
+        return -1;
     }
+    worklen -= ssl_cipher_suite_dig(decoder->cipher_suite)->len;
+    mac = out_str->data + worklen;
 
     /* If NULL encryption active and no keys are available, do not bother
      * checking the MAC. We do not have keys for that. */
diff --git a/epan/dissectors/packet-ssl-utils.h b/epan/dissectors/packet-ssl-utils.h
index 8760103922..8da1458a03 100644
--- a/epan/dissectors/packet-ssl-utils.h
+++ b/epan/dissectors/packet-ssl-utils.h
@@ -268,8 +268,9 @@ typedef enum {
     MODE_CCM_8      /* AEAD_AES_{128,256}_CCM with 8 byte auth tag */
 } ssl_cipher_mode_t;
 
-/* Explicit nonce length */
-#define SSL_EX_NONCE_LEN_GCM    8 /* RFC 5288 - section 3 */
+/* Explicit and implicit nonce length (RFC 5116 - Section 3.2.1) */
+#define IMPLICIT_NONCE_LEN  4
+#define EXPLICIT_NONCE_LEN  8
 
 #define SSL_DEBUG_USE_STDERR "-"
 
@@ -544,6 +545,7 @@ ssl_change_cipher(SslDecryptSession *ssl_session, gboolean server);
  @param ssl ssl_session the store all the session data
  @param decoder the stream decoder to be used
  @param ct the content type of this ssl record
+ @param record_version the version as contained in the record
  @param in a pointer to the ssl record to be decrypted
  @param inl the record length
  @param comp_str a pointer to the store the compression data
@@ -551,8 +553,8 @@ ssl_change_cipher(SslDecryptSession *ssl_session, gboolean server);
  @param outl the decrypted data len
  @return 0 on success */
 extern gint
-ssl_decrypt_record(SslDecryptSession* ssl,SslDecoder* decoder, gint ct,
-        const guchar* in, guint inl, StringInfo* comp_str, StringInfo* out_str, guint* outl);
+ssl_decrypt_record(SslDecryptSession *ssl, SslDecoder *decoder, guint8 ct, guint16 record_version,
+        const guchar *in, guint16 inl, StringInfo *comp_str, StringInfo *out_str, guint *outl);
 
 
 /* Common part bitween SSL and DTLS dissectors */
diff --git a/epan/dissectors/packet-ssl.c b/epan/dissectors/packet-ssl.c
index acd30d2f47..f73bcd45e8 100644
--- a/epan/dissectors/packet-ssl.c
+++ b/epan/dissectors/packet-ssl.c
@@ -881,8 +881,8 @@ dissect_ssl(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data _U_)
  * length "ssl_decrypted_data_avail".
  */
 static gboolean
-decrypt_ssl3_record(tvbuff_t *tvb, packet_info *pinfo, guint32 offset,
-        guint32 record_length, guint8 content_type, SslDecryptSession *ssl,
+decrypt_ssl3_record(tvbuff_t *tvb, packet_info *pinfo, guint32 offset, SslDecryptSession *ssl,
+        guint8 content_type, guint16 record_version, guint16 record_length,
         gboolean allow_fragments)
 {
     gboolean    success;
@@ -920,9 +920,9 @@ decrypt_ssl3_record(tvbuff_t *tvb, packet_info *pinfo, guint32 offset,
     /* run decryption and add decrypted payload to protocol data, if decryption
      * is successful*/
     ssl_decrypted_data_avail = ssl_decrypted_data.data_len;
-    success = ssl_decrypt_record(ssl, decoder,
-                           content_type, tvb_get_ptr(tvb, offset, record_length),
-                           record_length, &ssl_compressed_data, &ssl_decrypted_data, &ssl_decrypted_data_avail) == 0;
+    success = ssl_decrypt_record(ssl, decoder, content_type, record_version,
+                           tvb_get_ptr(tvb, offset, record_length), record_length,
+                           &ssl_compressed_data, &ssl_decrypted_data, &ssl_decrypted_data_avail) == 0;
     /*  */
     if (!success) {
         /* save data to update IV if valid session key is obtained later */
@@ -1523,7 +1523,7 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
      *    } TLSPlaintext;
      */
     guint32         record_length;
-    guint16         version;
+    guint16         record_version, version;
     guint8          content_type;
     guint8          next_byte;
     proto_tree     *ti;
@@ -1585,6 +1585,7 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
      */
     content_type  = tvb_get_guint8(tvb, offset);
     version       = tvb_get_ntohs(tvb, offset + 1);
+    record_version = version;
     record_length = tvb_get_ntohs(tvb, offset + 3);
 
     if (ssl_is_valid_content_type(content_type)) {
@@ -1695,7 +1696,8 @@ dissect_ssl3_record(tvbuff_t *tvb, packet_info *pinfo,
      * used as 'key' to identify this record in the packet (we can have multiple
      * handshake records in the same frame). */
     if (ssl) {
-        decrypt_ssl3_record(tvb, pinfo, offset, record_length, content_type, ssl,
+        decrypt_ssl3_record(tvb, pinfo, offset, ssl,
+                content_type, record_version, record_length,
                 content_type == SSL_ID_APP_DATA ||
                 content_type == SSL_ID_HANDSHAKE);
     }