Use the TLSv1.3 record header as AAD

As of TLSv1.3 draft-25 the record header data must be used as AAD

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/5604)

4 files changed, 47 insertions, 13 deletions

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 0953d2b01d..61010f4e72 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -825,7 +825,6 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
         thispkt = &pkt[j];
         thiswr = &wr[j];
 
-        SSL3_RECORD_set_type(thiswr, type);
         /*
          * In TLSv1.3, once encrypting, we always use application data for the
          * record type
@@ -834,6 +833,8 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
             rectype = SSL3_RT_APPLICATION_DATA;
         else
             rectype = type;
+        SSL3_RECORD_set_type(thiswr, rectype);
+
         /*
          * Some servers hang if initial client hello is larger than 256 bytes
          * and record version number > TLS 1.0
@@ -843,6 +844,7 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
                 && TLS1_get_version(s) > TLS1_VERSION
                 && s->hello_retry_request == SSL_HRR_NONE)
             version = TLS1_VERSION;
+        SSL3_RECORD_set_rec_version(thiswr, version);
 
         maxcomplen = pipelens[j];
         if (s->compress != NULL)
diff --git a/ssl/record/record_locl.h b/ssl/record/record_locl.h
index c20f5fec15..1782a4fa5b 100644
--- a/ssl/record/record_locl.h
+++ b/ssl/record/record_locl.h
@@ -80,6 +80,7 @@ int ssl3_release_write_buffer(SSL *s);
 
 #define SSL3_RECORD_get_type(r)                 ((r)->type)
 #define SSL3_RECORD_set_type(r, t)              ((r)->type = (t))
+#define SSL3_RECORD_set_rec_version(r, v)       ((r)->rec_version = (v))
 #define SSL3_RECORD_get_length(r)               ((r)->length)
 #define SSL3_RECORD_set_length(r, l)            ((r)->length = (l))
 #define SSL3_RECORD_add_length(r, l)            ((r)->length += (l))
diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
index fda918a9d8..5bfbaf982e 100644
--- a/ssl/record/ssl3_record.c
+++ b/ssl/record/ssl3_record.c
@@ -270,7 +270,8 @@ int ssl3_get_record(SSL *s)
                 thisrr->rec_version = version;
 
                 /*
-                 * Lets check version. In TLSv1.3 we ignore this field. For the
+                 * Lets check version. In TLSv1.3 we only check this field
+                 * when encryption is occurring (see later check). For the
                  * ServerHello after an HRR we haven't actually selected TLSv1.3
                  * yet, but we still treat it as TLSv1.3, so we must check for
                  * that explicitly
@@ -333,14 +334,19 @@ int ssl3_get_record(SSL *s)
                     }
                 }
 
-                if (SSL_IS_TLS13(s)
-                        && s->enc_read_ctx != NULL
-                        && thisrr->type != SSL3_RT_APPLICATION_DATA
-                        && (thisrr->type != SSL3_RT_CHANGE_CIPHER_SPEC
-                            || !SSL_IS_FIRST_HANDSHAKE(s))) {
-                    SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE,
-                             SSL_F_SSL3_GET_RECORD, SSL_R_BAD_RECORD_TYPE);
-                    return -1;
+                if (SSL_IS_TLS13(s) && s->enc_read_ctx != NULL) {
+                    if (thisrr->type != SSL3_RT_APPLICATION_DATA
+                            && (thisrr->type != SSL3_RT_CHANGE_CIPHER_SPEC
+                                || !SSL_IS_FIRST_HANDSHAKE(s))) {
+                        SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE,
+                                 SSL_F_SSL3_GET_RECORD, SSL_R_BAD_RECORD_TYPE);
+                        return -1;
+                    }
+                    if (thisrr->rec_version != TLS1_2_VERSION) {
+                        SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_SSL3_GET_RECORD,
+                                 SSL_R_WRONG_VERSION_NUMBER);
+                        return -1;
+                    }
                 }
 
                 if (thisrr->length >
diff --git a/ssl/record/ssl3_record_tls13.c b/ssl/record/ssl3_record_tls13.c
index f1e1667b9d..21073b637d 100644
--- a/ssl/record/ssl3_record_tls13.c
+++ b/ssl/record/ssl3_record_tls13.c
@@ -25,13 +25,14 @@
 int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
 {
     EVP_CIPHER_CTX *ctx;
-    unsigned char iv[EVP_MAX_IV_LENGTH];
-    size_t ivlen, taglen, offset, loop;
+    unsigned char iv[EVP_MAX_IV_LENGTH], recheader[SSL3_RT_HEADER_LENGTH];
+    size_t ivlen, taglen, offset, loop, hdrlen;
     unsigned char *staticiv;
     unsigned char *seq;
     int lenu, lenf;
     SSL3_RECORD *rec = &recs[0];
     uint32_t alg_enc;
+    WPACKET wpkt;
 
     if (n_recs != 1) {
         /* Should not happen */
@@ -143,7 +144,31 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
     if (EVP_CipherInit_ex(ctx, NULL, NULL, NULL, iv, sending) <= 0
             || (!sending && EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_TAG,
                                              taglen,
-                                             rec->data + rec->length) <= 0)
+                                             rec->data + rec->length) <= 0)) {
+        return -1;
+    }
+
+    /* Set up the AAD */
+    if (!WPACKET_init_static_len(&wpkt, recheader, sizeof(recheader), 0)
+            || !WPACKET_put_bytes_u8(&wpkt, rec->type)
+            || !WPACKET_put_bytes_u16(&wpkt, rec->rec_version)
+            || !WPACKET_put_bytes_u16(&wpkt, rec->length + taglen)
+            || !WPACKET_get_total_written(&wpkt, &hdrlen)
+            || hdrlen != SSL3_RT_HEADER_LENGTH
+            || !WPACKET_finish(&wpkt)) {
+        WPACKET_cleanup(&wpkt);
+        return -1;
+    }
+
+    /*
+     * For CCM we must explicitly set the total plaintext length before we add
+     * any AAD.
+     */
+    if (((alg_enc & SSL_AESCCM) != 0
+                 && EVP_CipherUpdate(ctx, NULL, &lenu, NULL,
+                                     (unsigned int)rec->length) <= 0)
+            || EVP_CipherUpdate(ctx, NULL, &lenu, recheader,
+                                sizeof(recheader)) <= 0
             || EVP_CipherUpdate(ctx, rec->data, &lenu, rec->input,
                                 (unsigned int)rec->length) <= 0
             || EVP_CipherFinal_ex(ctx, rec->data + lenu, &lenf) <= 0