Use a flag in SSL3_BUFFER to track when an application buffer is reused.

With KTLS, writes to an SSL connection store the application buffer
pointer directly in the 'buf' member instead of allocating a separate
buffer to hold the encrypted data.  As a result,
ssl3_release_write_buffer() has to avoid freeing these 'buf' pointers.

Previously, ssl3_release_write_buffer() checked for KTLS being enabled
on the write BIO to determine if a buffer should be freed.  However, a
buffer can outlive a BIO.  For example, 'openssl s_time' creates new
write BIOs when reusing sessions.  Since the new BIO did not have KTLS
enabled at the start of a connection, ssl3_release_write_buffer()
would incorrectly try to free the 'buf' pointer from the previous KTLS
connection.  To fix, track the state of 'buf' explicitly in
SSL3_BUFFER to determine if the 'buf' should be freed or simply
cleared.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/10489)

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index b4675f3c8c439458a04e7ef91bc0ffa72d8bc0ab..426da9ff1514d43cb18b3cc77fdf2b2413c1cf01 100644 (file)
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -771,6 +771,7 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
          */
         SSL3_BUFFER_set_buf(&s->rlayer.wbuf[0], (unsigned char *)buf);
         SSL3_BUFFER_set_offset(&s->rlayer.wbuf[0], 0);
+        SSL3_BUFFER_set_app_buffer(&s->rlayer.wbuf[0], 1);
         goto wpacket_init_complete;
     }
 

diff --git a/ssl/record/record.h b/ssl/record/record.h
index 784ab322a4c775cf4a001c71fcbb13fc995b0c46..f9d19246922ac0c8c7c45dab560cf4989994fa7f 100644 (file)
--- a/ssl/record/record.h
+++ b/ssl/record/record.h
@@ -25,6 +25,8 @@ typedef struct ssl3_buffer_st {
     size_t offset;
     /* how many bytes left */
     size_t left;
+    /* 'buf' is from application for KTLS */
+    int app_buffer;
 } SSL3_BUFFER;
 
 #define SEQ_NUM_SIZE                            8

diff --git a/ssl/record/record_local.h b/ssl/record/record_local.h
index ed421881d8633b1dc42b407ca59362b7739a6e5b..2f6a4c98a47cb79e395b8f2ff66b05e5edb35a04 100644 (file)
--- a/ssl/record/record_local.h
+++ b/ssl/record/record_local.h
@@ -65,6 +65,8 @@ void dtls1_record_bitmap_update(SSL *s, DTLS1_BITMAP *bitmap);
 #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);

diff --git a/ssl/record/ssl3_buffer.c b/ssl/record/ssl3_buffer.c
index bffe52194798b1dd42060a026894afff923d8eb4..ab977a5c1731e1f1ef55d0645aff9b933d5744ec 100644 (file)
--- a/ssl/record/ssl3_buffer.c
+++ b/ssl/record/ssl3_buffer.c
@@ -164,7 +164,9 @@ int ssl3_release_write_buffer(SSL *s)
     while (pipes > 0) {
         wb = &RECORD_LAYER_get_wbuf(&s->rlayer)[pipes - 1];
 
-        if (s->wbio == NULL || !BIO_get_ktls_send(s->wbio))
+        if (SSL3_BUFFER_is_app_buffer(wb))
+            SSL3_BUFFER_set_app_buffer(wb, 0);
+        else
             OPENSSL_free(wb->buf);
         wb->buf = NULL;
         pipes--;