Make SSL_alloc_buffers() and SSL_free_buffers() work again
authorMatt Caswell <matt@openssl.org>
Fri, 21 Oct 2022 15:12:31 +0000 (16:12 +0100)
committerMatt Caswell <matt@openssl.org>
Thu, 27 Oct 2022 09:52:52 +0000 (10:52 +0100)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19472)

ssl/record/methods/dtls_meth.c
ssl/record/methods/ktls_meth.c
ssl/record/methods/recmethod_local.h
ssl/record/methods/tls_common.c
ssl/record/rec_layer_s3.c
ssl/record/record.h
ssl/record/recordmethod.h
ssl/ssl_lib.c

index 1f3e1b102c9523e6cddc579d670c65edfe2dfd94..9988e2406f9d6cbb302fa788505a419a6f533277 100644 (file)
@@ -790,5 +790,7 @@ const OSSL_RECORD_METHOD ossl_dtls_record_method = {
     tls_get_compression,
     tls_set_max_frag_len,
     dtls_get_max_record_overhead,
-    tls_increment_sequence_ctr
+    tls_increment_sequence_ctr,
+    tls_alloc_buffers,
+    tls_free_buffers
 };
index 6354c85cd41a2dfe887fcad79cd50074f406e7cd..63cf6b475f6fd75013241ea92641cd8691c8b6b5 100644 (file)
@@ -533,6 +533,24 @@ static int ktls_prepare_write_bio(OSSL_RECORD_LAYER *rl, int type)
     return OSSL_RECORD_RETURN_SUCCESS;
 }
 
+static int ktls_alloc_buffers(OSSL_RECORD_LAYER *rl)
+{
+    /* We use the application buffer directly for writing */
+    if (rl->direction == OSSL_RECORD_DIRECTION_WRITE)
+        return 1;
+
+    return tls_alloc_buffers(rl);
+}
+
+static int ktls_free_buffers(OSSL_RECORD_LAYER *rl)
+{
+    /* We use the application buffer directly for writing */
+    if (rl->direction == OSSL_RECORD_DIRECTION_WRITE)
+        return 1;
+
+    return tls_free_buffers(rl);
+}
+
 static struct record_functions_st ossl_ktls_funcs = {
     ktls_set_crypto_state,
     ktls_cipher,
@@ -580,5 +598,7 @@ const OSSL_RECORD_METHOD ossl_ktls_record_method = {
     tls_get_compression,
     tls_set_max_frag_len,
     NULL,
-    tls_increment_sequence_ctr
+    tls_increment_sequence_ctr,
+    ktls_alloc_buffers,
+    ktls_free_buffers
 };
index 21a8297b085ea99ad65f962d669ae6bad1ab2c30..b02ab296bd276ad15a72d3a1347b997de85ec757 100644 (file)
@@ -345,6 +345,8 @@ __owur int ssl3_cbc_digest_record(const EVP_MD *md,
                                   size_t mac_secret_length, char is_sslv3);
 
 int tls_increment_sequence_ctr(OSSL_RECORD_LAYER *rl);
+int tls_alloc_buffers(OSSL_RECORD_LAYER *rl);
+int tls_free_buffers(OSSL_RECORD_LAYER *rl);
 
 int tls_default_read_n(OSSL_RECORD_LAYER *rl, size_t n, size_t max, int extend,
                        int clearold, size_t *readbytes);
index 96697828ba27d5e833a98365076b8592b9815235..4ef65ae1521e87543d29b67d2bc1eb5f71844020 100644 (file)
@@ -2031,6 +2031,66 @@ int tls_increment_sequence_ctr(OSSL_RECORD_LAYER *rl)
     return 1;
 }
 
+int tls_alloc_buffers(OSSL_RECORD_LAYER *rl)
+{
+    if (rl->direction == OSSL_RECORD_DIRECTION_WRITE) {
+        /* If we have a pending write then buffers are already allocated */
+        if (rl->nextwbuf < rl->numwpipes)
+            return 1;
+        /*
+         * We assume 1 pipe with default sized buffer. If what we need ends up
+         * being a different size to that then it will be reallocated on demand.
+         * If we need more than 1 pipe then that will also be allocated on
+         * demand
+         */
+        if (!tls_setup_write_buffer(rl, 1, 0, 0))
+            return 0;
+
+        /*
+         * Normally when we allocate write buffers we immediately write
+         * something into it. In this case we're not doing that so mark the
+         * buffer as empty.
+         */
+        SSL3_BUFFER_set_left(&rl->wbuf[0], 0);
+        return 1;
+    }
+
+    /* Read direction */
+
+    /* If we have pending data to be read then buffers are already allocated */
+    if (rl->curr_rec < rl->num_recs || SSL3_BUFFER_get_left(&rl->rbuf) != 0)
+        return 1;
+    return tls_setup_read_buffer(rl);
+}
+
+int tls_free_buffers(OSSL_RECORD_LAYER *rl)
+{
+    if (rl->direction == OSSL_RECORD_DIRECTION_WRITE) {
+        if (rl->nextwbuf < rl->numwpipes) {
+            /*
+             * We may have pending data. If we've just got one empty buffer
+             * allocated then it has probably just been alloc'd via
+             * tls_alloc_buffers, and it is fine to free it. Otherwise this
+             * looks like real pending data and it is an error.
+             */
+            if (rl->nextwbuf != 0
+                    || rl->numwpipes != 1
+                    || SSL3_BUFFER_get_left(&rl->wbuf[0]) != 0)
+                return 0;
+        }
+        tls_release_write_buffer(rl);
+        return 1;
+    }
+
+    /* Read direction */
+
+    /* If we have pending data to be read then fail */
+    if (rl->curr_rec < rl->num_recs || SSL3_BUFFER_get_left(&rl->rbuf) != 0)
+        return 0;
+
+    return tls_release_read_buffer(rl);
+}
+
 const OSSL_RECORD_METHOD ossl_tls_record_method = {
     tls_new_record_layer,
     tls_free,
@@ -2057,5 +2117,7 @@ const OSSL_RECORD_METHOD ossl_tls_record_method = {
     tls_get_compression,
     tls_set_max_frag_len,
     NULL,
-    tls_increment_sequence_ctr
+    tls_increment_sequence_ctr,
+    tls_alloc_buffers,
+    tls_free_buffers
 };
index 0e2c5b3b3f94fce64650dff0533dadf5c1a9d896..f90f639b0c0aca4884c614e523d3abf4f49a9524 100644 (file)
@@ -48,14 +48,6 @@ void RECORD_LAYER_clear(RECORD_LAYER *rl)
         DTLS_RECORD_LAYER_clear(rl);
 }
 
-void RECORD_LAYER_release(RECORD_LAYER *rl)
-{
-    /*
-     * 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 */
 int RECORD_LAYER_read_pending(const RECORD_LAYER *rl)
 {
index be86a758f5ec92848f918c8ba4c1eadb4cc4528f..d835703c1353706bf79fad467d0523be0c8da44a 100644 (file)
@@ -202,7 +202,6 @@ typedef struct ssl_mac_buf_st SSL_MAC_BUF;
 
 void RECORD_LAYER_init(RECORD_LAYER *rl, SSL_CONNECTION *s);
 void RECORD_LAYER_clear(RECORD_LAYER *rl);
-void RECORD_LAYER_release(RECORD_LAYER *rl);
 int RECORD_LAYER_read_pending(const RECORD_LAYER *rl);
 int RECORD_LAYER_processed_read_pending(const RECORD_LAYER *rl);
 int RECORD_LAYER_write_pending(const RECORD_LAYER *rl);
index 3bbca66fed029271ae2e0a7777b9d768974ff1fc..70e6e4d26adf5ed343fcf41a7e12f00bd3b2535f 100644 (file)
@@ -320,6 +320,18 @@ struct ossl_record_method_st {
      * Increment the record sequence number
      */
     int (*increment_sequence_ctr)(OSSL_RECORD_LAYER *rl);
+
+    /*
+     * Allocate read or write buffers. Does nothing if already allocated.
+     * Assumes default buffer length and 1 pipeline.
+     */
+    int (*alloc_buffers)(OSSL_RECORD_LAYER *rl);
+
+    /*
+     * Free read or write buffers. Fails if there is pending read or write
+     * data. Buffers are automatically reallocated on next read/write.
+     */
+    int (*free_buffers)(OSSL_RECORD_LAYER *rl);
 };
 
 
index 3b3eda4001d842cb86522c7fdf1dfb1bee6073e2..e71f0b5da5a0b33cab65322f69300f284bf239f9 100644 (file)
@@ -6365,22 +6365,22 @@ int SSL_free_buffers(SSL *ssl)
 
     rl = &sc->rlayer;
 
-    if (RECORD_LAYER_read_pending(rl) || RECORD_LAYER_write_pending(rl))
-        return 0;
-
-    RECORD_LAYER_release(rl);
-    return 1;
+    return rl->rrlmethod->free_buffers(rl->rrl)
+           && rl->wrlmethod->free_buffers(rl->wrl);
 }
 
 int SSL_alloc_buffers(SSL *ssl)
 {
+    RECORD_LAYER *rl;
     SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(ssl);
 
     if (sc == NULL)
         return 0;
 
-    /* TODO(RECLAYER): Need a way to make this happen in the record layer */
-    return 1;
+    rl = &sc->rlayer;
+
+    return rl->rrlmethod->alloc_buffers(rl->rrl)
+           && rl->wrlmethod->alloc_buffers(rl->wrl);
 }
 
 void SSL_CTX_set_keylog_callback(SSL_CTX *ctx, SSL_CTX_keylog_cb_func cb)