1 files changed, 33 insertions, 133 deletions

diff --git a/src/lib/libssl/ssl_pkt.c b/src/lib/libssl/ssl_pkt.c
index c9c86471d3..02a476ea82 100644
--- a/src/lib/libssl/ssl_pkt.c
+++ b/src/lib/libssl/ssl_pkt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_pkt.c,v 1.31 2020/08/30 15:40:20 jsing Exp $ */
+/* $OpenBSD: ssl_pkt.c,v 1.32 2020/10/03 17:35:16 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -149,15 +149,14 @@ ssl_force_want_read(SSL *s)
 static int
 ssl3_read_n(SSL *s, int n, int max, int extend)
 {
+        SSL3_BUFFER_INTERNAL *rb = &(S3I(s)->rbuf);
         int i, len, left;
         size_t align;
         unsigned char *pkt;
-        SSL3_BUFFER_INTERNAL *rb;
 
         if (n <= 0)
                 return n;
 
-        rb = &(S3I(s)->rbuf);
         if (rb->buf == NULL)
                 if (!ssl3_setup_read_buffer(s))
                         return -1;
@@ -327,15 +326,13 @@ ssl3_packet_extend(SSL *s, int plen)
 static int
 ssl3_get_record(SSL *s)
 {
-        int al;
-        int enc_err, n, i, ret = -1;
-        SSL3_RECORD_INTERNAL *rr;
-        SSL_SESSION *sess;
-        unsigned char md[EVP_MAX_MD_SIZE];
-        unsigned int mac_size, orig_len;
-
-        rr = &(S3I(s)->rrec);
-        sess = s->session;
+        SSL3_BUFFER_INTERNAL *rb = &(S3I(s)->rbuf);
+        SSL3_RECORD_INTERNAL *rr = &(S3I(s)->rrec);
+        uint8_t alert_desc;
+        uint8_t *out;
+        size_t out_len;
+        int al, n;
+        int ret = -1;
 
  again:
         /* check if we have the header */
@@ -387,17 +384,13 @@ ssl3_get_record(SSL *s)
                         goto err;
                 }
 
-                if (rr->length > S3I(s)->rbuf.len - SSL3_RT_HEADER_LENGTH) {
+                if (rr->length > rb->len - SSL3_RT_HEADER_LENGTH) {
                         al = SSL_AD_RECORD_OVERFLOW;
                         SSLerror(s, SSL_R_PACKET_LENGTH_TOO_LONG);
                         goto f_err;
                 }
-
-                /* now s->internal->rstate == SSL_ST_READ_BODY */
         }
 
-        /* s->internal->rstate == SSL_ST_READ_BODY, get and decode the data */
-
         n = ssl3_packet_extend(s, SSL3_RT_HEADER_LENGTH + rr->length);
         if (n <= 0)
                 return (n);
@@ -406,133 +399,40 @@ ssl3_get_record(SSL *s)
 
         s->internal->rstate = SSL_ST_READ_HEADER; /* set state for later operations */
 
-        /* At this point, s->internal->packet_length == SSL3_RT_HEADER_LNGTH + rr->length,
-         * and we have that many bytes in s->internal->packet
+        /*
+         * A full record has now been read from the wire, which now needs
+         * to be processed.
          */
-        rr->input = &(s->internal->packet[SSL3_RT_HEADER_LENGTH]);
-
-        /* ok, we can now read from 's->internal->packet' data into 'rr'
-         * rr->input points at rr->length bytes, which
-         * need to be copied into rr->data by either
-         * the decryption or by the decompression
-         * When the data is 'copied' into the rr->data buffer,
-         * rr->input will be pointed at the new buffer */
-
-        /* We now have - encrypted [ MAC [ compressed [ plain ] ] ]
-         * rr->length bytes of encrypted compressed stuff. */
-
-        /* check is not needed I believe */
-        if (rr->length > SSL3_RT_MAX_ENCRYPTED_LENGTH) {
-                al = SSL_AD_RECORD_OVERFLOW;
-                SSLerror(s, SSL_R_ENCRYPTED_LENGTH_TOO_LONG);
-                goto f_err;
-        }
-
-        /* decrypt in place in 'rr->input' */
-        rr->data = rr->input;
+        tls12_record_layer_set_version(s->internal->rl, s->version);
 
-        /* enc_err is:
-         *    0: (in non-constant time) if the record is publically invalid.
-         *    1: if the padding is valid
-         *    -1: if the padding is invalid */
-        if ((enc_err = tls1_enc(s, 0)) == 0) {
-                al = SSL_AD_BAD_RECORD_MAC;
-                SSLerror(s, SSL_R_BLOCK_CIPHER_PAD_IS_WRONG);
-                goto f_err;
-        }
-
-        /* r->length is now the compressed data plus mac */
-        if ((sess != NULL) && (s->enc_read_ctx != NULL) &&
-            (EVP_MD_CTX_md(s->read_hash) != NULL)) {
-                /* s->read_hash != NULL => mac_size != -1 */
-                unsigned char *mac = NULL;
-                unsigned char mac_tmp[EVP_MAX_MD_SIZE];
-
-                mac_size = EVP_MD_CTX_size(s->read_hash);
-                OPENSSL_assert(mac_size <= EVP_MAX_MD_SIZE);
-
-                orig_len = rr->length + rr->padding_length;
+        if (!tls12_record_layer_open_record(s->internal->rl, s->internal->packet,
+            s->internal->packet_length, &out, &out_len)) {
+                tls12_record_layer_alert(s->internal->rl, &alert_desc);
 
-                /* orig_len is the length of the record before any padding was
-                 * removed. This is public information, as is the MAC in use,
-                 * therefore we can safely process the record in a different
-                 * amount of time if it's too short to possibly contain a MAC.
-                 */
-                if (orig_len < mac_size ||
-                        /* CBC records must have a padding length byte too. */
-                    (EVP_CIPHER_CTX_mode(s->enc_read_ctx) == EVP_CIPH_CBC_MODE &&
-                    orig_len < mac_size + 1)) {
-                        al = SSL_AD_DECODE_ERROR;
-                        SSLerror(s, SSL_R_LENGTH_TOO_SHORT);
-                        goto f_err;
-                }
-
-                if (EVP_CIPHER_CTX_mode(s->enc_read_ctx) == EVP_CIPH_CBC_MODE) {
-                        /* We update the length so that the TLS header bytes
-                         * can be constructed correctly but we need to extract
-                         * the MAC in constant time from within the record,
-                         * without leaking the contents of the padding bytes.
-                         * */
-                        mac = mac_tmp;
-                        ssl3_cbc_copy_mac(mac_tmp, rr, mac_size, orig_len);
-                        rr->length -= mac_size;
-                } else {
-                        /* In this case there's no padding, so |orig_len|
-                         * equals |rec->length| and we checked that there's
-                         * enough bytes for |mac_size| above. */
-                        rr->length -= mac_size;
-                        mac = &rr->data[rr->length];
-                }
+                if (alert_desc == 0)
+                        goto err;
 
-                i = tls1_mac(s,md,0 /* not send */);
-                if (i < 0 || mac == NULL ||
-                    timingsafe_memcmp(md, mac, (size_t)mac_size) != 0)
-                        enc_err = -1;
-                if (rr->length >
-                    SSL3_RT_MAX_COMPRESSED_LENGTH + mac_size)
-                        enc_err = -1;
-        }
+                if (alert_desc == SSL_AD_RECORD_OVERFLOW)
+                        SSLerror(s, SSL_R_ENCRYPTED_LENGTH_TOO_LONG);
+                else if (alert_desc == SSL_AD_BAD_RECORD_MAC)
+                        SSLerror(s, SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC);
 
-        if (enc_err < 0) {
-                /*
-                 * A separate 'decryption_failed' alert was introduced with
-                 * TLS 1.0, SSL 3.0 only has 'bad_record_mac'. But unless a
-                 * decryption failure is directly visible from the ciphertext
-                 * anyway, we should not reveal which kind of error
-                 * occurred -- this might become visible to an attacker
-                 * (e.g. via a logfile)
-                 */
-                al = SSL_AD_BAD_RECORD_MAC;
-                SSLerror(s, SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC);
-                goto f_err;
-        }
-
-        if (rr->length > SSL3_RT_MAX_PLAIN_LENGTH) {
-                al = SSL_AD_RECORD_OVERFLOW;
-                SSLerror(s, SSL_R_DATA_LENGTH_TOO_LONG);
+                al = alert_desc;
                 goto f_err;
         }
 
+        rr->data = out;
+        rr->length = out_len;
         rr->off = 0;
-        /*
-         * So at this point the following is true
-         *
-         * ssl->s3->internal->rrec.type         is the type of record
-         * ssl->s3->internal->rrec.length       == number of bytes in record
-         * ssl->s3->internal->rrec.off  == offset to first valid byte
-         * ssl->s3->internal->rrec.data == where to take bytes from, increment
-         *                         after use :-).
-         */
 
         /* we have pulled in a full packet so zero things */
         s->internal->packet_length = 0;
 
         if (rr->length == 0) {
                 /*
-                 * CBC countermeasures for known IV weaknesses
-                 * can legitimately insert a single empty record,
-                 * so we allow ourselves to read once past a single
-                 * empty record without forcing want_read.
+                 * CBC countermeasures for known IV weaknesses can legitimately
+                 * insert a single empty record, so we allow ourselves to read
+                 * once past a single empty record without forcing want_read.
                  */
                 if (s->internal->empty_record_count++ > SSL_MAX_EMPTY_RECORDS) {
                         SSLerror(s, SSL_R_PEER_BEHAVING_BADLY);
@@ -543,15 +443,15 @@ ssl3_get_record(SSL *s)
                         return -1;
                 }
                 goto again;
-        } else {
-                s->internal->empty_record_count = 0;
         }
 
+        s->internal->empty_record_count = 0;
+
         return (1);
 
-f_err:
+ f_err:
         ssl3_send_alert(s, SSL3_AL_FATAL, al);
-err:
+ err:
         return (ret);
 }