Allow partially releasing a record for TLS

This enables the cleansing of plaintext to occur in the record layer and
avoids the need to cast away const above the record layer.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20404)

6 files changed, 107 insertions, 57 deletions

diff --git a/ssl/quic/quic_tls.c b/ssl/quic/quic_tls.c
index 62f8425d09..2dedc760cb 100644
--- a/ssl/quic/quic_tls.c
+++ b/ssl/quic/quic_tls.c
@@ -58,9 +58,12 @@ struct ossl_record_layer_st {
      */
     int alert;
 
-    /* Amount of crypto stream data we have received but not yet released  */
+    /* Amount of crypto stream data we read in the last call to quic_read_record */
     size_t recread;
 
+    /* Amount of crypto stream data read but not yet released */
+    size_t recunreleased;
+
     /* Callbacks */
     OSSL_FUNC_rlayer_msg_callback_fn *msg_callback;
     void *cbarg;
@@ -336,7 +339,7 @@ static int quic_read_record(OSSL_RECORD_LAYER *rl, void **rechandle,
                             size_t *datalen, uint16_t *epoch,
                             unsigned char *seq_num)
 {
-    if (rl->recread != 0)
+    if (rl->recread != 0 || rl->recunreleased != 0)
         return OSSL_RECORD_RETURN_FATAL;
 
     BIO_clear_retry_flags(rl->dummybio);
@@ -355,7 +358,7 @@ static int quic_read_record(OSSL_RECORD_LAYER *rl, void **rechandle,
     *rechandle = rl;
     *rversion = TLS1_3_VERSION;
     *type = SSL3_RT_HANDSHAKE;
-    rl->recread = *datalen;
+    rl->recread = rl->recunreleased = *datalen;
     /* epoch/seq_num are not relevant for TLS */
 
     if (rl->msg_callback != NULL) {
@@ -386,22 +389,30 @@ static int quic_read_record(OSSL_RECORD_LAYER *rl, void **rechandle,
     return OSSL_RECORD_RETURN_SUCCESS;
 }
 
-static int quic_release_record(OSSL_RECORD_LAYER *rl, void *rechandle)
+static int quic_release_record(OSSL_RECORD_LAYER *rl, void *rechandle,
+                               size_t length)
 {
-    if (!ossl_assert(rl->recread > 0) || !ossl_assert(rl == rechandle)) {
+    if (!ossl_assert(rl->recread > 0)
+            || !ossl_assert(rl->recunreleased <= rl->recread)
+            || !ossl_assert(rl == rechandle)
+            || !ossl_assert(length <= rl->recunreleased)) {
         QUIC_TLS_FATAL(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
-        return 0;
+        return OSSL_RECORD_RETURN_FATAL;
     }
 
+    rl->recunreleased -= length;
+
+    if (rl->recunreleased > 0)
+        return OSSL_RECORD_RETURN_SUCCESS;
+
     if (!rl->qtls->args.crypto_release_rcd_cb(rl->recread,
                                               rl->qtls->args.crypto_release_rcd_cb_arg)) {
         QUIC_TLS_FATAL(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         return OSSL_RECORD_RETURN_FATAL;
     }
 
-
     rl->recread = 0;
-    return 1;
+    return OSSL_RECORD_RETURN_SUCCESS;
 }
 
 static int quic_get_alert_code(OSSL_RECORD_LAYER *rl)
diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h
index e6310908d8..9454ca56b8 100644
--- a/ssl/record/methods/recmethod_local.h
+++ b/ssl/record/methods/recmethod_local.h
@@ -461,7 +461,7 @@ int tls_set1_bio(OSSL_RECORD_LAYER *rl, BIO *bio);
 int tls_read_record(OSSL_RECORD_LAYER *rl, void **rechandle, int *rversion,
                     int *type, const unsigned char **data, size_t *datalen,
                     uint16_t *epoch, unsigned char *seq_num);
-int tls_release_record(OSSL_RECORD_LAYER *rl, void *rechandle);
+int tls_release_record(OSSL_RECORD_LAYER *rl, void *rechandle, size_t length);
 int tls_default_set_protocol_version(OSSL_RECORD_LAYER *rl, int version);
 int tls_set_protocol_version(OSSL_RECORD_LAYER *rl, int version);
 void tls_set_plain_alerts(OSSL_RECORD_LAYER *rl, int allow);
diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c
index 32865fdbf1..a93bd91daf 100644
--- a/ssl/record/methods/tls_common.c
+++ b/ssl/record/methods/tls_common.c
@@ -1132,15 +1132,32 @@ int tls_read_record(OSSL_RECORD_LAYER *rl, void **rechandle, int *rversion,
     return OSSL_RECORD_RETURN_SUCCESS;
 }
 
-int tls_release_record(OSSL_RECORD_LAYER *rl, void *rechandle)
+int tls_release_record(OSSL_RECORD_LAYER *rl, void *rechandle, size_t length)
 {
+    TLS_RL_RECORD *rec = &rl->rrec[rl->num_released];
+
     if (!ossl_assert(rl->num_released < rl->curr_rec)
-            || !ossl_assert(rechandle == &rl->rrec[rl->num_released])) {
+            || !ossl_assert(rechandle == rec)) {
         /* Should not happen */
         RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_INVALID_RECORD);
         return OSSL_RECORD_RETURN_FATAL;
     }
 
+    if (rec->length < length) {
+        /* Should not happen */
+        RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+        return OSSL_RECORD_RETURN_FATAL;
+    }
+
+    if ((rl->options & SSL_OP_CLEANSE_PLAINTEXT) != 0)
+        OPENSSL_cleanse(rec->data + rec->off, length);
+
+    rec->off += length;
+    rec->length -= length;
+
+    if (rec->length > 0)
+        return OSSL_RECORD_RETURN_SUCCESS;
+
     rl->num_released++;
 
     if (rl->curr_rec == rl->num_released
diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c
index 6a2d128807..fed57b65cd 100644
--- a/ssl/record/rec_layer_d1.c
+++ b/ssl/record/rec_layer_d1.c
@@ -293,7 +293,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
             /* SSLfatal() already called */
             return -1;
         }
-        ssl_release_record(sc, rr);
+        if (!ssl_release_record(sc, rr, 0))
+            return -1;
         goto start;
     }
 
@@ -302,7 +303,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
      * 'peek' mode)
      */
     if (sc->shutdown & SSL_RECEIVED_SHUTDOWN) {
-        ssl_release_record(sc, rr);
+        if (!ssl_release_record(sc, rr, 0))
+            return -1;
         sc->rwstate = SSL_NOTHING;
         return 0;
     }
@@ -335,8 +337,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
              * SSL_read() with a zero length buffer will eventually cause
              * SSL_pending() to report data as being available.
              */
-            if (rr->length == 0)
-                ssl_release_record(sc, rr);
+            if (rr->length == 0 && !ssl_release_record(sc, rr, 0))
+                return -1;
             return 0;
         }
 
@@ -347,16 +349,11 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
 
         memcpy(buf, &(rr->data[rr->off]), n);
         if (peek) {
-            if (rr->length == 0)
-                ssl_release_record(sc, rr);
+            if (rr->length == 0 && !ssl_release_record(sc, rr, 0))
+                return -1;
         } else {
-            /* TODO(RECLAYER): Casting away the const is wrong! FIX ME */
-            if (sc->options & SSL_OP_CLEANSE_PLAINTEXT)
-                OPENSSL_cleanse((unsigned char *)&(rr->data[rr->off]), n);
-            rr->length -= n;
-            rr->off += n;
-            if (rr->length == 0)
-                ssl_release_record(sc, rr);
+            if (!ssl_release_record(sc, rr, n))
+                return -1;
         }
 #ifndef OPENSSL_NO_SCTP
         /*
@@ -409,7 +406,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
 
         if (alert_level == SSL3_AL_WARNING) {
             sc->s3.warn_alert = alert_descr;
-            ssl_release_record(sc, rr);
+            if (!ssl_release_record(sc, rr, 0))
+                return -1;
 
             sc->rlayer.alert_count++;
             if (sc->rlayer.alert_count == MAX_WARN_ALERT_COUNT) {
@@ -444,7 +442,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
                           SSL_AD_REASON_OFFSET + alert_descr,
                           "SSL alert number %d", alert_descr);
             sc->shutdown |= SSL_RECEIVED_SHUTDOWN;
-            ssl_release_record(sc, rr);
+            if (!ssl_release_record(sc, rr, 0))
+                return -1;
             SSL_CTX_remove_session(sc->session_ctx, sc->session);
             return 0;
         } else {
@@ -458,7 +457,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
     if (sc->shutdown & SSL_SENT_SHUTDOWN) { /* but we have not received a
                                             * shutdown */
         sc->rwstate = SSL_NOTHING;
-        ssl_release_record(sc, rr);
+        if (!ssl_release_record(sc, rr, 0))
+            return -1;
         return 0;
     }
 
@@ -467,7 +467,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
          * We can't process a CCS now, because previous handshake messages
          * are still missing, so just drop it.
          */
-        ssl_release_record(sc, rr);
+        if (!ssl_release_record(sc, rr, 0))
+            return -1;
         goto start;
     }
 
@@ -483,7 +484,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
          */
         if (rr->epoch != sc->rlayer.d->r_epoch
                 || rr->length < DTLS1_HM_HEADER_LENGTH) {
-            ssl_release_record(sc, rr);
+            if (!ssl_release_record(sc, rr, 0))
+                return -1;
             goto start;
         }
 
@@ -504,7 +506,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
                 if (ossl_statem_in_error(sc))
                     return -1;
             }
-            ssl_release_record(sc, rr);
+            if (!ssl_release_record(sc, rr, 0))
+                return -1;
             if (!(sc->mode & SSL_MODE_AUTO_RETRY)) {
                 if (!sc->rlayer.rrlmethod->unprocessed_read_pending(sc->rlayer.rrl)) {
                     /* no read-ahead left? */
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index c567b471ea..e59588472f 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -10,6 +10,7 @@
 #include <stdio.h>
 #include <limits.h>
 #include <errno.h>
+#include <assert.h>
 #include "../ssl_local.h"
 #include <openssl/evp.h>
 #include <openssl/buffer.h>
@@ -496,17 +497,35 @@ int ossl_tls_handle_rlayer_return(SSL_CONNECTION *s, int writing, int ret,
     return ret;
 }
 
-void ssl_release_record(SSL_CONNECTION *s, TLS_RECORD *rr)
+int ssl_release_record(SSL_CONNECTION *s, TLS_RECORD *rr, size_t length)
 {
+    assert(rr->length >= length);
     if (rr->rechandle != NULL) {
+        if (length == 0)
+            length = rr->length;
         /* The record layer allocated the buffers for this record */
-        s->rlayer.rrlmethod->release_record(s->rlayer.rrl, rr->rechandle);
-    } else {
+        if (HANDLE_RLAYER_READ_RETURN(s,
+                s->rlayer.rrlmethod->release_record(s->rlayer.rrl,
+                                                    rr->rechandle,
+                                                    length)) <= 0) {
+            /* RLAYER_fatal already called */
+            return 0;
+        }
+
+        if (length == rr->length)
+            s->rlayer.curr_rec++;
+    } else if (length == 0 || length == rr->length) {
         /* We allocated the buffers for this record (only happens with DTLS) */
         OPENSSL_free(rr->allocdata);
         rr->allocdata = NULL;
     }
-    s->rlayer.curr_rec++;
+    rr->length -= length;
+    if (rr->length > 0)
+        rr->off += length;
+    else
+        rr->off = 0;
+
+    return 1;
 }
 
 /*-
@@ -700,8 +719,8 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
              * SSL_read() with a zero length buffer will eventually cause
              * SSL_pending() to report data as being available.
              */
-            if (rr->length == 0)
-                ssl_release_record(s, rr);
+            if (rr->length == 0 && !ssl_release_record(s, rr, 0))
+                return -1;
 
             return 0;
         }
@@ -718,16 +737,11 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
             buf += n;
             if (peek) {
                 /* Mark any zero length record as consumed CVE-2016-6305 */
-                if (rr->length == 0)
-                    ssl_release_record(s, rr);
+                if (rr->length == 0 && !ssl_release_record(s, rr, 0))
+                    return -1;
             } else {
-                /* TODO(RECLAYER) Casting away the const here is wrong! FIX ME */
-                if (s->options & SSL_OP_CLEANSE_PLAINTEXT)
-                    OPENSSL_cleanse((unsigned char *)&(rr->data[rr->off]), n);
-                rr->length -= n;
-                rr->off += n;
-                if (rr->length == 0)
-                    ssl_release_record(s, rr);
+                if (!ssl_release_record(s, rr, n))
+                    return -1;
             }
             if (rr->length == 0
                 || (peek && n == rr->length)) {
@@ -814,7 +828,8 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
         if ((!is_tls13 && alert_level == SSL3_AL_WARNING)
                 || (is_tls13 && alert_descr == SSL_AD_USER_CANCELLED)) {
             s->s3.warn_alert = alert_descr;
-            ssl_release_record(s, rr);
+            if (!ssl_release_record(s, rr, 0))
+                return -1;
 
             s->rlayer.alert_count++;
             if (s->rlayer.alert_count == MAX_WARN_ALERT_COUNT) {
@@ -841,7 +856,8 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
                           SSL_AD_REASON_OFFSET + alert_descr,
                           "SSL alert number %d", alert_descr);
             s->shutdown |= SSL_RECEIVED_SHUTDOWN;
-            ssl_release_record(s, rr);
+            if (!ssl_release_record(s, rr, 0))
+                return -1;
             SSL_CTX_remove_session(s->session_ctx, s->session);
             return 0;
         } else if (alert_descr == SSL_AD_NO_RENEGOTIATION) {
@@ -876,7 +892,8 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
              * sent close_notify.
              */
             if (!SSL_CONNECTION_IS_TLS13(s)) {
-                ssl_release_record(s, rr);
+                if (!ssl_release_record(s, rr, 0))
+                    return -1;
 
                 if ((s->mode & SSL_MODE_AUTO_RETRY) != 0)
                     goto start;
@@ -895,7 +912,8 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
              * above.
              * No alert sent because we already sent close_notify
              */
-            ssl_release_record(s, rr);
+            if (!ssl_release_record(s, rr, 0))
+                return -1;
             SSLfatal(s, SSL_AD_NO_ALERT,
                      SSL_R_APPLICATION_DATA_AFTER_CLOSE_NOTIFY);
             return -1;
@@ -918,12 +936,12 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
             n = rr->length; /* available bytes */
 
         /* now move 'n' bytes: */
-        memcpy(dest + *dest_len, rr->data + rr->off, n);
-        rr->off += n;
-        rr->length -= n;
-        *dest_len += n;
-        if (rr->length == 0)
-            ssl_release_record(s, rr);
+        if (n > 0) {
+            memcpy(dest + *dest_len, rr->data + rr->off, n);
+            *dest_len += n;
+            if (!ssl_release_record(s, rr, n))
+                return -1;
+        }
 
         if (*dest_len < dest_maxlen)
             goto start;     /* fragment was too small */
@@ -1027,7 +1045,8 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
                 /* SSLfatal() already called */
                 return -1;
             }
-            ssl_release_record(s, rr);
+            if (!ssl_release_record(s, rr, 0))
+                return -1;
             goto start;
         } else {
             SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_R_UNEXPECTED_RECORD);
diff --git a/ssl/record/record.h b/ssl/record/record.h
index a277f09e18..7dcbbb36e9 100644
--- a/ssl/record/record.h
+++ b/ssl/record/record.h
@@ -165,7 +165,7 @@ __owur int dtls1_write_bytes(SSL_CONNECTION *s, int type, const void *buf,
 int do_dtls1_write(SSL_CONNECTION *s, int type, const unsigned char *buf,
                    size_t len, size_t *written);
 void dtls1_increment_epoch(SSL_CONNECTION *s, int rw);
-void ssl_release_record(SSL_CONNECTION *s, TLS_RECORD *rr);
+int ssl_release_record(SSL_CONNECTION *s, TLS_RECORD *rr, size_t length);
 
 # define HANDLE_RLAYER_READ_RETURN(s, ret) \
     ossl_tls_handle_rlayer_return(s, 0, ret, OPENSSL_FILE, OPENSSL_LINE)