Remove the old buffer management code

We no longer use the old buffer management code now that it has all been
moved to the new record layer.

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

ssl/d1_lib.c

@@ -454,14 +454,12 @@ int DTLSv1_listen(SSL *ssl, BIO_ADDR *client)
         return -1;
     }
 
-    if (!ssl3_setup_buffers(s)) {
-        /* ERR_raise() already called */
-        return -1;
-    }
     buf = OPENSSL_malloc(DTLS1_RT_HEADER_LENGTH + SSL3_RT_MAX_PLAIN_LENGTH);
     if (buf == NULL)
         return -1;
-    wbuf = RECORD_LAYER_get_wbuf(&s->rlayer)[0].buf;
+    wbuf = OPENSSL_malloc(DTLS1_RT_HEADER_LENGTH + SSL3_RT_MAX_PLAIN_LENGTH);
+    if (buf == NULL)
+        return -1;
 
     do {
         /* Get a packet */
@@ -836,6 +834,7 @@ int DTLSv1_listen(SSL *ssl, BIO_ADDR *client)
  end:
     BIO_ADDR_free(tmpclient);
     OPENSSL_free(buf);
+    OPENSSL_free(wbuf);
     return ret;
 }
 #endif

ssl/record/build.info

@@ -11,7 +11,7 @@ IF[{- !$disabled{asm} -}]
 ENDIF
 
 SOURCE[../../libssl]=\
-        rec_layer_s3.c rec_layer_d1.c ssl3_buffer.c  ssl3_record.c \
+        rec_layer_s3.c rec_layer_d1.c ssl3_record.c \
         ssl3_record_tls13.c
 
 # For shared builds we need to include the sources needed in providers

ssl/record/methods/recmethod_local.h

@@ -460,3 +460,19 @@ int tls_post_encryption_processing_default(OSSL_RECORD_LAYER *rl,
 int tls_write_records_default(OSSL_RECORD_LAYER *rl,
                               OSSL_RECORD_TEMPLATE *templates,
                               size_t numtempl);
+
+/* Macros/functions provided by the SSL3_BUFFER component */
+
+#define SSL3_BUFFER_get_buf(b)              ((b)->buf)
+#define SSL3_BUFFER_set_buf(b, n)           ((b)->buf = (n))
+#define SSL3_BUFFER_get_len(b)              ((b)->len)
+#define SSL3_BUFFER_get_left(b)             ((b)->left)
+#define SSL3_BUFFER_set_left(b, l)          ((b)->left = (l))
+#define SSL3_BUFFER_sub_left(b, l)          ((b)->left -= (l))
+#define SSL3_BUFFER_get_offset(b)           ((b)->offset)
+#define SSL3_BUFFER_set_offset(b, o)        ((b)->offset = (o))
+#define SSL3_BUFFER_add_offset(b, o)        ((b)->offset += (o))
+#define SSL3_BUFFER_set_app_buffer(b, l)    ((b)->app_buffer = (l))
+#define SSL3_BUFFER_is_app_buffer(b)        ((b)->app_buffer)
+
+void SSL3_BUFFER_release(SSL3_BUFFER *b);

ssl/record/methods/tls_common.c

@@ -22,6 +22,12 @@
 
 static void tls_int_free(OSSL_RECORD_LAYER *rl);
 
+void SSL3_BUFFER_release(SSL3_BUFFER *b)
+{
+    OPENSSL_free(b->buf);
+    b->buf = NULL;
+}
+
 void ossl_rlayer_fatal(OSSL_RECORD_LAYER *rl, int al, int reason,
                        const char *fmt, ...)
 {

ssl/record/rec_layer_d1.c

@@ -639,21 +639,8 @@ int do_dtls1_write(SSL_CONNECTION *sc, int type, const unsigned char *buf,
     int i;
     OSSL_RECORD_TEMPLATE tmpl;
     SSL *s = SSL_CONNECTION_GET_SSL(sc);
-    SSL3_BUFFER *wb;
     int ret;
 
-    wb = &sc->rlayer.wbuf[0];
-
-    /*
-     * DTLS writes whole datagrams, so there can't be anything left in
-     * the buffer.
-     */
-    /* TODO(RECLAYER): Remove me */
-    if (!ossl_assert(SSL3_BUFFER_get_left(wb) == 0)) {
-        SSLfatal(sc, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
-        return 0;
-    }
-
     /* If we have an alert to send, lets send it */
     if (sc->s3.alert_dispatch) {
         i = s->method->ssl_dispatch_alert(s);

ssl/record/rec_layer_s3.c

@@ -33,8 +33,6 @@ void RECORD_LAYER_clear(RECORD_LAYER *rl)
     rl->wpend_ret = 0;
     rl->wpend_buf = NULL;
 
-    ssl3_release_write_buffer(rl->s);
-
     RECORD_LAYER_reset_write_sequence(rl);
 
     if (rl->rrlmethod != NULL)
@@ -54,8 +52,10 @@ void RECORD_LAYER_clear(RECORD_LAYER *rl)
 
 void RECORD_LAYER_release(RECORD_LAYER *rl)
 {
-    if (rl->numwpipes > 0)
-        ssl3_release_write_buffer(rl->s);
+    /*
+     * TODO(RECLAYER): Need a way to release the write buffers in the record
+     * layer on demand
+     */
 }
 
 /* Checks if we have unprocessed read ahead data pending */
@@ -73,10 +73,6 @@ int RECORD_LAYER_processed_read_pending(const RECORD_LAYER *rl)
 
 int RECORD_LAYER_write_pending(const RECORD_LAYER *rl)
 {
-    /* 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;
 }
 

ssl/record/record.h

@@ -21,7 +21,7 @@ typedef struct ssl3_buffer_st SSL3_BUFFER;
  *****************************************************************************/
 
 struct ssl3_buffer_st {
-    /* at least SSL3_RT_MAX_PACKET_SIZE bytes, see ssl3_setup_buffers() */
+    /* at least SSL3_RT_MAX_PACKET_SIZE bytes */
     unsigned char *buf;
     /* default buffer size (or 0 if no default set) */
     size_t default_len;
@@ -149,14 +149,7 @@ typedef struct record_layer_st {
      * non-blocking reads)
      */
     int read_ahead;
-    /*
-     * TODO(RECLAYER): These next 2 fields can be removed when DTLS is moved to
-     * the new write record layer architecture.
-     */
-    /* How many pipelines can be used to write data */
-    size_t numwpipes;
-    /* write IO goes into here */
-    SSL3_BUFFER wbuf[SSL_MAX_PIPELINES + 1];
+
     /* number of bytes sent so far */
     size_t wnum;
     unsigned char handshake_fragment[4];
@@ -225,7 +218,6 @@ __owur int ssl3_write_bytes(SSL *s, int type, const void *buf, size_t len,
 __owur int ssl3_read_bytes(SSL *s, int type, int *recvd_type,
                            unsigned char *buf, size_t len, int peek,
                            size_t *readbytes);
-__owur int ssl3_setup_buffers(SSL_CONNECTION *s);
 __owur int tls1_enc(SSL_CONNECTION *s, SSL3_RECORD *recs, size_t n_recs,
                     int sending, SSL_MAC_BUF *mac, size_t macsize);
 __owur int tls1_mac_old(SSL_CONNECTION *s, SSL3_RECORD *rec, unsigned char *md,

ssl/record/record_local.h

@@ -27,30 +27,6 @@
 
 int dtls_buffer_record(SSL_CONNECTION *s, TLS_RECORD *rec);
 
-/* Macros/functions provided by the SSL3_BUFFER component */
-
-#define SSL3_BUFFER_get_buf(b)              ((b)->buf)
-#define SSL3_BUFFER_set_buf(b, n)           ((b)->buf = (n))
-#define SSL3_BUFFER_get_len(b)              ((b)->len)
-#define SSL3_BUFFER_set_len(b, l)           ((b)->len = (l))
-#define SSL3_BUFFER_get_left(b)             ((b)->left)
-#define SSL3_BUFFER_set_left(b, l)          ((b)->left = (l))
-#define SSL3_BUFFER_sub_left(b, l)          ((b)->left -= (l))
-#define SSL3_BUFFER_get_offset(b)           ((b)->offset)
-#define SSL3_BUFFER_set_offset(b, o)        ((b)->offset = (o))
-#define SSL3_BUFFER_add_offset(b, o)        ((b)->offset += (o))
-#define SSL3_BUFFER_is_initialised(b)       ((b)->buf != NULL)
-#define SSL3_BUFFER_set_default_len(b, l)   ((b)->default_len = (l))
-#define SSL3_BUFFER_set_app_buffer(b, l)    ((b)->app_buffer = (l))
-#define SSL3_BUFFER_is_app_buffer(b)        ((b)->app_buffer)
-
-void SSL3_BUFFER_clear(SSL3_BUFFER *b);
-void SSL3_BUFFER_set_data(SSL3_BUFFER *b, const unsigned char *d, size_t n);
-void SSL3_BUFFER_release(SSL3_BUFFER *b);
-__owur int ssl3_setup_write_buffer(SSL_CONNECTION *s, size_t numwpipes,
-                                   size_t len);
-int ssl3_release_write_buffer(SSL_CONNECTION *s);
-
 /* Macros/functions provided by the SSL3_RECORD component */
 
 #define SSL3_RECORD_get_type(r)                 ((r)->type)

ssl/record/ssl3_buffer.c

@@ -33,102 +33,3 @@ void SSL3_BUFFER_release(SSL3_BUFFER *b)
     OPENSSL_free(b->buf);
     b->buf = NULL;
 }
-
-int ssl3_setup_write_buffer(SSL_CONNECTION *s, size_t numwpipes, size_t len)
-{
-    unsigned char *p;
-    size_t align = 0, headerlen;
-    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) {
-        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 (!(s->options & SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS))
-            len += headerlen + align + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD;
-    }
-
-    wb = RECORD_LAYER_get_wbuf(&s->rlayer);
-    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_CRYPTO_LIB);
-                    return 0;
-                }
-            } else {
-                p = NULL;
-            }
-            memset(thiswb, 0, sizeof(SSL3_BUFFER));
-            thiswb->buf = p;
-            thiswb->len = len;
-        }
-    }
-
-    return 1;
-}
-
-int ssl3_setup_buffers(SSL_CONNECTION *s)
-{
-    if (!ssl3_setup_write_buffer(s, 1, 0)) {
-        /* SSLfatal() already called */
-        return 0;
-    }
-    return 1;
-}
-
-int ssl3_release_write_buffer(SSL_CONNECTION *s)
-{
-    SSL3_BUFFER *wb;
-    size_t pipes;
-
-    pipes = s->rlayer.numwpipes;
-    while (pipes > 0) {
-        wb = &RECORD_LAYER_get_wbuf(&s->rlayer)[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--;
-    }
-    s->rlayer.numwpipes = 0;
-    return 1;
-}

ssl/ssl_lib.c

@@ -6388,7 +6388,8 @@ int SSL_alloc_buffers(SSL *ssl)
     if (sc == NULL)
         return 0;
 
-    return ssl3_setup_buffers(sc);
+    /* TODO(RECLAYER): Need a way to make this happen in the record layer */
+    return 1;
 }
 
 void SSL_CTX_set_keylog_callback(SSL_CTX *ctx, SSL_CTX_keylog_cb_func cb)

ssl/ssl_local.h

@@ -2447,7 +2447,6 @@ const SSL_METHOD *func_name(void)  \
 
 struct openssl_ssl_test_functions {
     int (*p_ssl_init_wbio_buffer) (SSL_CONNECTION *s);
-    int (*p_ssl3_setup_buffers) (SSL_CONNECTION *s);
 };
 
 const char *ssl_protocol_to_string(int version);
@@ -2959,7 +2958,6 @@ void ssl_session_calculate_timeout(SSL_SESSION *ss);
 # else /* OPENSSL_UNIT_TEST */
 
 #  define ssl_init_wbio_buffer SSL_test_functions()->p_ssl_init_wbio_buffer
-#  define ssl3_setup_buffers SSL_test_functions()->p_ssl3_setup_buffers
 
 # endif
 

ssl/ssl_utst.c

@@ -13,7 +13,6 @@
 
 static const struct openssl_ssl_test_functions ssl_test_functions = {
     ssl_init_wbio_buffer,
-    ssl3_setup_buffers,
 };
 
 const struct openssl_ssl_test_functions *SSL_test_functions(void)

ssl/statem/extensions.c

@@ -1732,12 +1732,6 @@ static int final_maxfragmentlen(SSL_CONNECTION *s, unsigned int context,
                                               GET_MAX_FRAGMENT_LENGTH(s->session));
         s->rlayer.wrlmethod->set_max_frag_len(s->rlayer.wrl,
                                               ssl_get_max_send_fragment(s));
-        /* trigger a larger buffer reallocation */
-        /* TODO(RECLAYER): Remove me when DTLS moved to write record layer */
-        if (SSL_CONNECTION_IS_DTLS(s) && !ssl3_setup_buffers(s)) {
-            /* SSLfatal() already called */
-            return 0;
-        }
     }
 
     return 1;

ssl/statem/statem.c

@@ -439,10 +439,6 @@ static int state_machine(SSL_CONNECTION *s, int server)
             buf = NULL;
         }
 
-        if (!ssl3_setup_buffers(s)) {
-            SSLfatal(s, SSL_AD_NO_ALERT, ERR_R_INTERNAL_ERROR);
-            goto end;
-        }
         s->init_num = 0;
 
         /*