Move write buffer management into the write record layer

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19198)

diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h
index 16f24b6d5e9f2fe9daee0f5bcd26708cc0a66922..475fb7c047fa2d4829f6f4e9eb5578677478f274 100644 (file)
--- a/ssl/record/methods/recmethod_local.h
+++ b/ssl/record/methods/recmethod_local.h
@@ -96,6 +96,12 @@ struct ossl_record_layer_st
     uint64_t options;
     uint32_t mode;
 
+    /* write IO goes into here */
+    SSL3_BUFFER wbuf[SSL_MAX_PIPELINES + 1];
+
+    /* Next wbuf with pending data still to write */
+    size_t nextwbuf;
+
     /* read IO goes into here */
     SSL3_BUFFER rbuf;
     /* each decoded record goes in here */

diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c
index 19b34dd1a0448e8aa39426a790ce52ea32904981..5e1f1bb01b15cd64e8848c58b2b88b3f53d75ca4 100644 (file)
--- a/ssl/record/methods/tls_common.c
+++ b/ssl/record/methods/tls_common.c
@@ -94,6 +94,94 @@ static int tls_allow_compression(OSSL_RECORD_LAYER *rl)
 }
 #endif
 
+static int tls_setup_write_buffer(OSSL_RECORD_LAYER *rl, size_t numwpipes,
+                                  size_t len)
+{
+    unsigned char *p;
+    size_t align = 0, headerlen;
+    SSL3_BUFFER *wb;
+    size_t currpipe;
+    SSL_CONNECTION *s = (SSL_CONNECTION *)rl->cbarg;
+
+    s->rlayer.numwpipes = numwpipes;
+
+    if (len == 0) {
+        if (SSL_CONNECTION_IS_DTLS(s))
+            headerlen = DTLS1_RT_HEADER_LENGTH + 1;
+        else
+            headerlen = SSL3_RT_HEADER_LENGTH;
+
+#if defined(SSL3_ALIGN_PAYLOAD) && SSL3_ALIGN_PAYLOAD!=0
+        align = SSL3_ALIGN_PAYLOAD - 1;
+#endif
+
+        len = ssl_get_max_send_fragment(s)
+            + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD + headerlen + align;
+#ifndef OPENSSL_NO_COMP
+        if (ssl_allow_compression(s))
+            len += SSL3_RT_MAX_COMPRESSED_OVERHEAD;
+#endif
+        if (!(rl->options & SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS))
+            len += headerlen + align + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD;
+    }
+
+    wb = rl->wbuf;
+    for (currpipe = 0; currpipe < numwpipes; currpipe++) {
+        SSL3_BUFFER *thiswb = &wb[currpipe];
+
+        if (thiswb->len != len) {
+            OPENSSL_free(thiswb->buf);
+            thiswb->buf = NULL;         /* force reallocation */
+        }
+
+        if (thiswb->buf == NULL) {
+            if (s->wbio == NULL || !BIO_get_ktls_send(s->wbio)) {
+                p = OPENSSL_malloc(len);
+                if (p == NULL) {
+                    s->rlayer.numwpipes = currpipe;
+                    /*
+                     * We've got a malloc failure, and we're still initialising
+                     * buffers. We assume we're so doomed that we won't even be able
+                     * to send an alert.
+                     */
+                    SSLfatal(s, SSL_AD_NO_ALERT, ERR_R_MALLOC_FAILURE);
+                    return 0;
+                }
+            } else {
+                p = NULL;
+            }
+            memset(thiswb, 0, sizeof(SSL3_BUFFER));
+            thiswb->buf = p;
+            thiswb->len = len;
+        }
+    }
+
+    return 1;
+}
+
+static void tls_release_write_buffer(OSSL_RECORD_LAYER *rl)
+{
+    SSL3_BUFFER *wb;
+    size_t pipes;
+    SSL_CONNECTION *s = (SSL_CONNECTION *)rl->cbarg;
+
+    pipes = (s == NULL) ? 0 : s->rlayer.numwpipes;
+
+    while (pipes > 0) {
+        wb = &rl->wbuf[pipes - 1];
+
+        if (SSL3_BUFFER_is_app_buffer(wb))
+            SSL3_BUFFER_set_app_buffer(wb, 0);
+        else
+            OPENSSL_free(wb->buf);
+        wb->buf = NULL;
+        pipes--;
+    }
+    /* TODO(RECLAYER): REMOVE ME */
+    if (rl->direction == OSSL_RECORD_DIRECTION_WRITE)
+        s->rlayer.numwpipes = 0;
+}
+
 int tls_setup_read_buffer(OSSL_RECORD_LAYER *rl)
 {
     unsigned char *p;
@@ -1223,6 +1311,8 @@ static void tls_int_free(OSSL_RECORD_LAYER *rl)
     BIO_free(rl->next);
     SSL3_BUFFER_release(&rl->rbuf);
 
+    tls_release_write_buffer(rl);
+
     EVP_CIPHER_CTX_free(rl->enc_ctx);
     EVP_MD_CTX_free(rl->md_ctx);
 #ifndef OPENSSL_NO_COMP
@@ -1324,7 +1414,9 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
     OSSL_RECORD_TEMPLATE prefixtempl;
     OSSL_RECORD_TEMPLATE *thistempl;
 
-    if (!ossl_assert(!RECORD_LAYER_write_pending(&s->rlayer))) {
+    /* Check we don't have pending data waiting to write */
+    if (!ossl_assert(rl->nextwbuf >= s->rlayer.numwpipes
+                     || SSL3_BUFFER_get_left(&rl->wbuf[rl->nextwbuf]) == 0)) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
         goto err;
     }
@@ -1359,7 +1451,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
          * TODO(RECLAYER): In the prefix case the first buffer can be a lot
          * smaller. It is wasteful to allocate a full sized buffer here
          */
-        if (!ssl3_setup_write_buffer(s, numtempl + prefix, 0)) {
+        if (!tls_setup_write_buffer(rl, numtempl + prefix, 0)) {
             /* SSLfatal() already called */
             return -1;
         }
@@ -1384,7 +1476,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
         /* TODO(RECLAYER): Do we actually need this? */
         s->s3.empty_fragment_done = 1;
 
-        wb = &s->rlayer.wbuf[0];
+        wb = &rl->wbuf[0];
         /* TODO(RECLAYER): This alignment calculation no longer seems right */
 #if defined(SSL3_ALIGN_PAYLOAD) && SSL3_ALIGN_PAYLOAD!=0
         /*
@@ -1407,7 +1499,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
     for (j = 0; j < numtempl; j++) {
         thispkt = &pkt[prefix + j];
 
-        wb = &s->rlayer.wbuf[prefix + j];
+        wb = &rl->wbuf[prefix + j];
         wb->type = templates[j].type;
 
         if (using_ktls) {
@@ -1749,9 +1841,10 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
         SSL3_RECORD_set_type(thiswr, thistempl->type);
 
         /* now let's set up wb */
-        SSL3_BUFFER_set_left(&s->rlayer.wbuf[j], SSL3_RECORD_get_length(thiswr));
+        SSL3_BUFFER_set_left(&rl->wbuf[j], SSL3_RECORD_get_length(thiswr));
     }
 
+    rl->nextwbuf = 0;
     /* we now just need to write the buffers */
     return tls_retry_write_records(rl);
  err:
@@ -1768,18 +1861,15 @@ int tls_retry_write_records(OSSL_RECORD_LAYER *rl)
 {
     int i;
     SSL3_BUFFER *thiswb;
-    size_t currbuf = 0;
     size_t tmpwrit = 0;
     SSL_CONNECTION *s = rl->cbarg;
 
+    if (rl->nextwbuf >= s->rlayer.numwpipes)
+        return 1;
+
     for (;;) {
-        thiswb = &s->rlayer.wbuf[currbuf];
-        /* Loop until we find a buffer we haven't written out yet */
-        if (SSL3_BUFFER_get_left(thiswb) == 0
-            && currbuf < s->rlayer.numwpipes - 1) {
-            currbuf++;
-            continue;
-        }
+        thiswb = &rl->wbuf[rl->nextwbuf];
+
         clear_sys_error();
         if (rl->bio != NULL) {
             s->rwstate = SSL_WRITING;
@@ -1816,7 +1906,7 @@ int tls_retry_write_records(OSSL_RECORD_LAYER *rl)
         if (i >= 0 && tmpwrit == SSL3_BUFFER_get_left(thiswb)) {
             SSL3_BUFFER_set_left(thiswb, 0);
             SSL3_BUFFER_add_offset(thiswb, tmpwrit);
-            if (currbuf + 1 < s->rlayer.numwpipes)
+            if (++(rl->nextwbuf) < s->rlayer.numwpipes)
                 continue;
             s->rwstate = SSL_NOTHING;
             /*
@@ -1824,6 +1914,11 @@ int tls_retry_write_records(OSSL_RECORD_LAYER *rl)
              * in ciphersuites with known-IV weakness:
              */
             s->s3.empty_fragment_done = 0;
+
+            if (rl->nextwbuf == s->rlayer.numwpipes
+                    && (rl->mode & SSL_MODE_RELEASE_BUFFERS) != 0)
+                tls_release_write_buffer(rl);
+
             return 1;
         } else if (i <= 0) {
             if (SSL_CONNECTION_IS_DTLS(s)) {
@@ -1832,6 +1927,10 @@ int tls_retry_write_records(OSSL_RECORD_LAYER *rl)
                  * using a datagram service
                  */
                 SSL3_BUFFER_set_left(thiswb, 0);
+                if (++(rl->nextwbuf) == s->rlayer.numwpipes
+                        && (rl->mode & SSL_MODE_RELEASE_BUFFERS) != 0)
+                    tls_release_write_buffer(rl);
+
             }
             return i;
         }

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index fcad40456c18c084b5eeed65a0b7cfc0420585d6..1da4f6a1263ba8eddd11a921ecf0d7b622c06e21 100644 (file)
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -82,8 +82,11 @@ int RECORD_LAYER_processed_read_pending(const RECORD_LAYER *rl)
 
 int RECORD_LAYER_write_pending(const RECORD_LAYER *rl)
 {
-    return (rl->numwpipes > 0)
-        && SSL3_BUFFER_get_left(&rl->wbuf[rl->numwpipes - 1]) != 0;
+    /* TODO(RECLAYER): Remove me when DTLS is moved to the write record layer */
+    if (SSL_CONNECTION_IS_DTLS(rl->s))
+        return (rl->numwpipes > 0)
+            && SSL3_BUFFER_get_left(&rl->wbuf[rl->numwpipes - 1]) != 0;
+    return rl->wpend_tot > 0;
 }
 
 void RECORD_LAYER_reset_write_sequence(RECORD_LAYER *rl)
@@ -198,7 +201,6 @@ int ssl3_write_bytes(SSL *ssl, int type, const void *buf_, size_t len,
 #if 0 && !defined(OPENSSL_NO_MULTIBLOCK) && EVP_CIPH_FLAG_TLS1_1_MULTIBLOCK
     size_t nw;
 #endif
-    SSL3_BUFFER *wb;
     int i;
     SSL_CONNECTION *s = SSL_CONNECTION_FROM_SSL_ONLY(ssl);
     OSSL_RECORD_TEMPLATE tmpls[SSL_MAX_PIPELINES];
@@ -206,7 +208,6 @@ int ssl3_write_bytes(SSL *ssl, int type, const void *buf_, size_t len,
     if (s == NULL)
         return -1;
 
-    wb = &s->rlayer.wbuf[0];
     s->rwstate = SSL_NOTHING;
     tot = s->rlayer.wnum;
     /*
@@ -219,7 +220,8 @@ int ssl3_write_bytes(SSL *ssl, int type, const void *buf_, size_t len,
      * report the error in a way the user will notice
      */
     if ((len < s->rlayer.wnum)
-        || ((wb->left != 0) && (len < (s->rlayer.wnum + s->rlayer.wpend_tot)))) {
+        || ((s->rlayer.wpend_tot != 0)
+            && (len < (s->rlayer.wnum + s->rlayer.wpend_tot)))) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_BAD_LENGTH);
         return -1;
     }
@@ -237,8 +239,8 @@ int ssl3_write_bytes(SSL *ssl, int type, const void *buf_, size_t len,
      * into init unless we have writes pending - in which case we should finish
      * doing that first.
      */
-    if (wb->left == 0 && (s->key_update != SSL_KEY_UPDATE_NONE
-                          || s->ext.extra_tickets_expected > 0))
+    if (s->rlayer.wpend_tot == 0 && (s->key_update != SSL_KEY_UPDATE_NONE
+                                     || s->ext.extra_tickets_expected > 0))
         ossl_statem_set_in_init(s, 1);
 
     /*
@@ -417,9 +419,6 @@ int ssl3_write_bytes(SSL *ssl, int type, const void *buf_, size_t len,
     } else
 #endif  /* !defined(OPENSSL_NO_MULTIBLOCK) && EVP_CIPH_FLAG_TLS1_1_MULTIBLOCK */
     if (tot == len) {           /* done? */
-        if (s->mode & SSL_MODE_RELEASE_BUFFERS && !SSL_CONNECTION_IS_DTLS(s))
-            ssl3_release_write_buffer(s);
-
         *written = tot;
         return 1;
     }
@@ -532,14 +531,9 @@ int ssl3_write_bytes(SSL *ssl, int type, const void *buf_, size_t len,
             return i;
         }
 
-        if (s->rlayer.wpend_tot == n ||
-            (type == SSL3_RT_APPLICATION_DATA &&
-             (s->mode & SSL_MODE_ENABLE_PARTIAL_WRITE))) {
-            if (s->rlayer.wpend_tot == n
-                    && (s->mode & SSL_MODE_RELEASE_BUFFERS) != 0
-                    && !SSL_CONNECTION_IS_DTLS(s))
-                ssl3_release_write_buffer(s);
-
+        if (s->rlayer.wpend_tot == n
+                || (type == SSL3_RT_APPLICATION_DATA
+                    && (s->mode & SSL_MODE_ENABLE_PARTIAL_WRITE) != 0)) {
             *written = tot + s->rlayer.wpend_tot;
             s->rlayer.wpend_tot = 0;
             return 1;

diff --git a/ssl/record/ssl3_buffer.c b/ssl/record/ssl3_buffer.c
index 1955148b1aa0731c3df91be47ce4f7cf18ceab8b..98ef8e65ecb3ef1440993e0eef0336c00725e496 100644 (file)
--- a/ssl/record/ssl3_buffer.c
+++ b/ssl/record/ssl3_buffer.c
@@ -41,6 +41,13 @@ int ssl3_setup_write_buffer(SSL_CONNECTION *s, size_t numwpipes, size_t len)
     SSL3_BUFFER *wb;
     size_t currpipe;
 
+    /*
+     * TODO(RECLAYER): Eventually this function can be removed once everything
+     * is moved to the write record layer.
+     */
+    if (!SSL_CONNECTION_IS_DTLS(s))
+        return 1;
+
     s->rlayer.numwpipes = numwpipes;
 
     if (len == 0) {

diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c
index 7c7876b7df8de69f3477c3c67ff0977945f45a3e..2ef0da41b58afac860d85bb29640824361a43786 100644 (file)
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -437,12 +437,8 @@ int tls1_change_cipher_state(SSL_CONNECTION *s, int which)
         goto skip_ktls;
 
     /* ktls works with user provided buffers directly */
-    if (BIO_set_ktls(bio, &crypto_info, which & SSL3_CC_WRITE)) {
-        if (which & SSL3_CC_WRITE)
-            ssl3_release_write_buffer(s);
+    if (BIO_set_ktls(bio, &crypto_info, which & SSL3_CC_WRITE))
         SSL_set_options(SSL_CONNECTION_GET_SSL(s), SSL_OP_NO_RENEGOTIATION);
-    }
-
 #endif                          /* OPENSSL_NO_KTLS */
  skip_ktls:
     s->statem.enc_write_state = ENC_WRITE_STATE_VALID;

diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c
index 829dfe3c102c4dad7423c4029c8f6c3b357237ed..1e8e189b7c91199d96ad38fa154d61d90108af60 100644 (file)
--- a/ssl/tls13_enc.c
+++ b/ssl/tls13_enc.c
@@ -769,12 +769,6 @@ int tls13_change_cipher_state(SSL_CONNECTION *s, int which)
                                rl_sequence, &crypto_info, which & SSL3_CC_WRITE,
                                iv, ivlen, key, keylen, NULL, 0))
         goto skip_ktls;
-
-    /* ktls works with user provided buffers directly */
-    if (BIO_set_ktls(bio, &crypto_info, which & SSL3_CC_WRITE)) {
-        if (which & SSL3_CC_WRITE)
-            ssl3_release_write_buffer(s);
-    }
 # endif
 #endif
 skip_ktls: