diff --git a/backport-CVE-2024-4741-Extend-the-SSL_free_buffers-t-3.patch b/backport-CVE-2024-4741-Extend-the-SSL_free_buffers-t-3.patch new file mode 100644 index 0000000000000000000000000000000000000000..b2b81a71bab6ef47f7e54cc835671ae8db97a434 --- /dev/null +++ b/backport-CVE-2024-4741-Extend-the-SSL_free_buffers-t-3.patch @@ -0,0 +1,127 @@ +From b0ee93ce4ec51cdf0550354dc01dd9303b248dcd Mon Sep 17 00:00:00 2001 +From: Matt Caswell +Date: Wed, 29 May 2024 09:43:54 +0800 +Subject: [PATCH] Extend the SSL_free_buffers testing + +Test that attempting to free the buffers at points where they should not +be freed works as expected. + +Follow on from CVE-2024-4741 +--- + test/sslbuffertest.c | 93 ++++++++++++++++++++++++++++++++++++++++++++ + 1 file changed, 93 insertions(+) + +diff --git a/test/sslbuffertest.c b/test/sslbuffertest.c +index 3c3e69d..133fdb1 100644 +--- a/test/sslbuffertest.c ++++ b/test/sslbuffertest.c +@@ -150,6 +150,98 @@ static int test_func(int test) + return result; + } + ++/* ++ * Test that attempting to free the buffers at points where they cannot be freed ++ * works as expected ++ * Test 0: Attempt to free buffers after a full record has been processed, but ++ * the application has only performed a partial read ++ * Test 1: Attempt to free buffers after only a partial record header has been ++ * received ++ * Test 2: Attempt to free buffers after a full record header but no record body ++ * Test 3: Attempt to free buffers after a full record hedaer and partial record ++ * body ++ */ ++static int test_free_buffers(int test) ++{ ++ int result = 0; ++ SSL *serverssl = NULL, *clientssl = NULL; ++ const char testdata[] = "Test data"; ++ char buf[40]; ++ size_t written, readbytes; ++ ++ if (!TEST_true(create_ssl_objects(serverctx, clientctx, &serverssl, ++ &clientssl, NULL, NULL))) ++ goto end; ++ ++ if (!TEST_true(create_ssl_connection(serverssl, clientssl, ++ SSL_ERROR_NONE))) ++ goto end; ++ ++ ++ if (!TEST_true(SSL_write_ex(clientssl, testdata, sizeof(testdata), ++ &written))) ++ goto end; ++ ++ if (test == 0) { ++ /* ++ * Deliberately only read the first byte - so the remaining bytes are ++ * still buffered ++ */ ++ if (!TEST_true(SSL_read_ex(serverssl, buf, 1, &readbytes))) ++ goto end; ++ } else { ++ BIO *tmp; ++ size_t partial_len; ++ ++ /* Remove all the data that is pending for read by the server */ ++ tmp = SSL_get_rbio(serverssl); ++ if (!TEST_true(BIO_read_ex(tmp, buf, sizeof(buf), &readbytes)) ++ || !TEST_size_t_lt(readbytes, sizeof(buf)) ++ || !TEST_size_t_gt(readbytes, SSL3_RT_HEADER_LENGTH)) ++ goto end; ++ ++ switch(test) { ++ case 1: ++ partial_len = SSL3_RT_HEADER_LENGTH - 1; ++ break; ++ case 2: ++ partial_len = SSL3_RT_HEADER_LENGTH; ++ break; ++ case 3: ++ partial_len = readbytes - 1; ++ break; ++ default: ++ TEST_error("Invalid test index"); ++ goto end; ++ } ++ ++ /* Put back just the partial record */ ++ if (!TEST_true(BIO_write_ex(tmp, buf, partial_len, &written))) ++ goto end; ++ ++ /* ++ * Attempt a read. This should fail because only a partial record is ++ * available. ++ */ ++ if (!TEST_false(SSL_read_ex(serverssl, buf, 1, &readbytes))) ++ goto end; ++ } ++ ++ /* ++ * Attempting to free the buffers at this point should fail because they are ++ * still in use ++ */ ++ if (!TEST_false(SSL_free_buffers(serverssl))) ++ goto end; ++ ++ result = 1; ++ end: ++ SSL_free(clientssl); ++ SSL_free(serverssl); ++ ++ return result; ++} ++ + OPT_TEST_DECLARE_USAGE("certfile privkeyfile\n") + + int setup_tests(void) +@@ -173,6 +265,7 @@ int setup_tests(void) + } + + ADD_ALL_TESTS(test_func, 9); ++ ADD_ALL_TESTS(test_free_buffers, 4); + return 1; + } + +-- +2.33.0 + diff --git a/backport-CVE-2024-4741-Further-extend-the-SSL_free_b-5.patch b/backport-CVE-2024-4741-Further-extend-the-SSL_free_b-5.patch new file mode 100644 index 0000000000000000000000000000000000000000..c955d1c09b8dab1890a7c0daedd12ff6de278f84 --- /dev/null +++ b/backport-CVE-2024-4741-Further-extend-the-SSL_free_b-5.patch @@ -0,0 +1,160 @@ +From 1153eda04cf4f0e046dbf5ee0ba8b17d5c612165 Mon Sep 17 00:00:00 2001 +From: Matt Caswell +Date: Wed, 29 May 2024 10:13:17 +0800 +Subject: [PATCH] Further extend the SSL_free_buffers testing + +We extend the testing to test what happens when pipelining is in use. + +Follow on from CVE-2024-4741 +--- + test/sslbuffertest.c | 96 +++++++++++++++++++++++++++++++++++++------- + 1 file changed, 81 insertions(+), 15 deletions(-) + +diff --git a/test/sslbuffertest.c b/test/sslbuffertest.c +index 133fdb1..aad770c 100644 +--- a/test/sslbuffertest.c ++++ b/test/sslbuffertest.c +@@ -160,34 +160,65 @@ static int test_func(int test) + * Test 2: Attempt to free buffers after a full record header but no record body + * Test 3: Attempt to free buffers after a full record hedaer and partial record + * body ++ * Test 4-7: We repeat tests 0-3 but including data from a second pipelined ++ * record + */ + static int test_free_buffers(int test) + { + int result = 0; + SSL *serverssl = NULL, *clientssl = NULL; + const char testdata[] = "Test data"; +- char buf[40]; ++ char buf[120]; + size_t written, readbytes; ++ int pipeline = test > 3; ++ ENGINE *e = NULL; ++ ++ if (pipeline) { ++ e = load_dasync(); ++ if (e == NULL) ++ goto end; ++ test -= 4; ++ } + + if (!TEST_true(create_ssl_objects(serverctx, clientctx, &serverssl, + &clientssl, NULL, NULL))) + goto end; + ++ if (pipeline) { ++ if (!TEST_true(SSL_set_cipher_list(serverssl, "AES128-SHA")) ++ || !TEST_true(SSL_set_max_proto_version(serverssl, ++ TLS1_2_VERSION)) ++ || !TEST_true(SSL_set_max_pipelines(serverssl, 2))) ++ goto end; ++ } ++ + if (!TEST_true(create_ssl_connection(serverssl, clientssl, + SSL_ERROR_NONE))) + goto end; + +- +- if (!TEST_true(SSL_write_ex(clientssl, testdata, sizeof(testdata), +- &written))) +- goto end; ++ /* ++ * For the non-pipeline case we write one record. For pipelining we write ++ * two records. ++ */ ++ for (int i = 0; i <= pipeline; i++) { ++ if (!TEST_true(SSL_write_ex(clientssl, testdata, strlen(testdata), ++ &written))) ++ goto end; ++ } + + if (test == 0) { ++ size_t readlen = 1; ++ + /* +- * Deliberately only read the first byte - so the remaining bytes are +- * still buffered +- */ +- if (!TEST_true(SSL_read_ex(serverssl, buf, 1, &readbytes))) ++ * Deliberately only read the first byte - so the remaining bytes are ++ * still buffered. In the pipelining case we read as far as the first ++ * byte from the second record. ++ */ ++ if (pipeline) ++ readlen += strlen(testdata); ++ ++ if (!TEST_true(SSL_read_ex(serverssl, buf, readlen, &readbytes)) ++ || !TEST_size_t_eq(readlen, readbytes)) + goto end; + } else { + BIO *tmp; +@@ -215,16 +246,47 @@ static int test_free_buffers(int test) + goto end; + } + +- /* Put back just the partial record */ +- if (!TEST_true(BIO_write_ex(tmp, buf, partial_len, &written))) +- goto end; ++ if (pipeline) { ++ /* We happen to know the first record is 57 bytes long */ ++ const size_t first_rec_len = 57; ++ ++ if (test != 3) ++ partial_len += first_rec_len; ++ ++ /* ++ * Sanity check. If we got the record len right then this should ++ * never fail. ++ */ ++ if (!TEST_int_eq(buf[first_rec_len], SSL3_RT_APPLICATION_DATA)) ++ goto end; ++ } + + /* +- * Attempt a read. This should fail because only a partial record is +- * available. ++ * Put back just the partial record (plus the whole initial record in ++ * the pipelining case) + */ +- if (!TEST_false(SSL_read_ex(serverssl, buf, 1, &readbytes))) ++ if (!TEST_true(BIO_write_ex(tmp, buf, partial_len, &written))) + goto end; ++ ++ if (pipeline) { ++ /* ++ * Attempt a read. This should pass but only return data from the ++ * first record. Only a partial record is available for the second ++ * record. ++ */ ++ if (!TEST_true(SSL_read_ex(serverssl, buf, sizeof(buf), ++ &readbytes)) ++ || !TEST_size_t_eq(readbytes, strlen(testdata))) ++ goto end; ++ } else { ++ /* ++ * Attempt a read. This should fail because only a partial record is ++ * available. ++ */ ++ if (!TEST_false(SSL_read_ex(serverssl, buf, sizeof(buf), ++ &readbytes))) ++ goto end; ++ } + } + + /* +@@ -265,7 +327,11 @@ int setup_tests(void) + } + + ADD_ALL_TESTS(test_func, 9); ++#if !defined(OPENSSL_NO_TLS1_2) && !defined(OPENSSL_NO_DYNAMIC_ENGINE) ++ ADD_ALL_TESTS(test_free_buffers, 8); ++#else + ADD_ALL_TESTS(test_free_buffers, 4); ++#endif + return 1; + } + +-- +2.33.0 + diff --git a/backport-CVE-2024-4741-Move-the-ability-to-load-the-4.patch b/backport-CVE-2024-4741-Move-the-ability-to-load-the-4.patch new file mode 100644 index 0000000000000000000000000000000000000000..9e7ad1c1b443654a6348e1b901c983bda9851c7f --- /dev/null +++ b/backport-CVE-2024-4741-Move-the-ability-to-load-the-4.patch @@ -0,0 +1,79 @@ +From 9656aaafe2228df17a5393228f72afe5f9840ce1 Mon Sep 17 00:00:00 2001 +From: Matt Caswell +Date: Wed, 29 May 2024 09:50:06 +0800 +Subject: [PATCH] Move the ability to load the dasync engine into ssltestlib.c + +The sslapitest has a helper function to load the dasync engine which is +useful for testing pipelining. We would like to have the same facility +from sslbuffertest, so we move the function to the common location +ssltestlib.c + +Follow on from CVE-2024-4741 +--- + test/helpers/ssltestlib.c | 33 +++++++++++++++++++++++++++++++++ + test/helpers/ssltestlib.h | 1 + + 2 files changed, 34 insertions(+) + +diff --git a/test/helpers/ssltestlib.c b/test/helpers/ssltestlib.c +index ef4a617..da14f66 100644 +--- a/test/helpers/ssltestlib.c ++++ b/test/helpers/ssltestlib.c +@@ -7,8 +7,17 @@ + * https://www.openssl.org/source/license.html + */ + ++/* ++ * We need access to the deprecated low level ENGINE APIs for legacy purposes ++ * when the deprecated calls are not hidden ++ */ ++#ifndef OPENSSL_NO_DEPRECATED_3_0 ++# define OPENSSL_SUPPRESS_DEPRECATED ++#endif ++ + #include + ++#include + #include "internal/nelem.h" + #include "ssltestlib.h" + #include "../testutil.h" +@@ -1182,3 +1191,27 @@ void shutdown_ssl_connection(SSL *serverssl, SSL *clientssl) + SSL_free(serverssl); + SSL_free(clientssl); + } ++ ++ENGINE *load_dasync(void) ++{ ++#if !defined(OPENSSL_NO_TLS1_2) && !defined(OPENSSL_NO_DYNAMIC_ENGINE) ++ ENGINE *e; ++ ++ if (!TEST_ptr(e = ENGINE_by_id("dasync"))) ++ return NULL; ++ ++ if (!TEST_true(ENGINE_init(e))) { ++ ENGINE_free(e); ++ return NULL; ++ } ++ ++ if (!TEST_true(ENGINE_register_ciphers(e))) { ++ ENGINE_free(e); ++ return NULL; ++ } ++ ++ return e; ++#else ++ return NULL; ++#endif ++} +diff --git a/test/helpers/ssltestlib.h b/test/helpers/ssltestlib.h +index 8e9daa5..2777fb3 100644 +--- a/test/helpers/ssltestlib.h ++++ b/test/helpers/ssltestlib.h +@@ -59,4 +59,5 @@ typedef struct mempacket_st MEMPACKET; + + DEFINE_STACK_OF(MEMPACKET) + ++ENGINE *load_dasync(void); + #endif /* OSSL_TEST_SSLTESTLIB_H */ +-- +2.33.0 + diff --git a/backport-CVE-2024-4741-Only-free-the-read-buffers-if-1.patch b/backport-CVE-2024-4741-Only-free-the-read-buffers-if-1.patch new file mode 100644 index 0000000000000000000000000000000000000000..d500ede93f254879081c32660fe533bf5624a4cb --- /dev/null +++ b/backport-CVE-2024-4741-Only-free-the-read-buffers-if-1.patch @@ -0,0 +1,65 @@ +From 8011033b459ee5bd362941f150714da503fa2867 Mon Sep 17 00:00:00 2001 +From: Watson Ladd +Date: Wed, 29 May 2024 09:35:26 +0800 +Subject: [PATCH] Only free the read buffers if we're not using them + +If we're part way through processing a record, or the application has +not released all the records then we should not free our buffer because +they are still needed. + +CVE-2024-4741 +--- + ssl/record/rec_layer_s3.c | 9 +++++++++ + ssl/record/record.h | 1 + + ssl/ssl_lib.c | 3 +++ + 3 files changed, 13 insertions(+) + +diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c +index 3baf820..99602b6 100644 +--- a/ssl/record/rec_layer_s3.c ++++ b/ssl/record/rec_layer_s3.c +@@ -81,6 +81,15 @@ int RECORD_LAYER_read_pending(const RECORD_LAYER *rl) + return SSL3_BUFFER_get_left(&rl->rbuf) != 0; + } + ++int RECORD_LAYER_data_present(const RECORD_LAYER *rl) ++{ ++ if (rl->rstate == SSL_ST_READ_BODY) ++ return 1; ++ if (RECORD_LAYER_processed_read_pending(rl)) ++ return 1; ++ return 0; ++} ++ + /* Checks if we have decrypted unread record data pending */ + int RECORD_LAYER_processed_read_pending(const RECORD_LAYER *rl) + { +diff --git a/ssl/record/record.h b/ssl/record/record.h +index 234656b..b60f71c 100644 +--- a/ssl/record/record.h ++++ b/ssl/record/record.h +@@ -205,6 +205,7 @@ 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); ++int RECORD_LAYER_data_present(const RECORD_LAYER *rl); + void RECORD_LAYER_reset_read_sequence(RECORD_LAYER *rl); + void RECORD_LAYER_reset_write_sequence(RECORD_LAYER *rl); + int RECORD_LAYER_is_sslv2_record(RECORD_LAYER *rl); +diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c +index 92bfaa3..846c55a 100644 +--- a/ssl/ssl_lib.c ++++ b/ssl/ssl_lib.c +@@ -5472,6 +5472,9 @@ int SSL_free_buffers(SSL *ssl) + if (RECORD_LAYER_read_pending(rl) || RECORD_LAYER_write_pending(rl)) + return 0; + ++ if (RECORD_LAYER_data_present(rl)) ++ return 0; ++ + RECORD_LAYER_release(rl); + return 1; + } +-- +2.33.0 + diff --git a/backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-aft-2.patch b/backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-aft-2.patch new file mode 100644 index 0000000000000000000000000000000000000000..697ab1a010d7dc408a5199b280cb136190fcef6e --- /dev/null +++ b/backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-aft-2.patch @@ -0,0 +1,46 @@ +From 1c4f067ce96197730b01a7374a2e908801fc6f4aMon Sep 17 00:00:00 2001 +From: Matt Caswell +Date: Wed, 29 May 2024 09:39:11 +0800 +Subject: [PATCH] Set rlayer.packet to NULL after we've finished using it + +In order to ensure we do not have a UAF we reset the rlayer.packet pointer +to NULL after we free it. + +CVE-2024-4741 +--- + ssl/record/rec_layer_s3.c | 6 ++++++ + ssl/record/ssl3_buffer.c | 2 ++ + 2 files changed, 8 insertions(+) + +diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c +index 99602b6..b8def97 100644 +--- a/ssl/record/rec_layer_s3.c ++++ b/ssl/record/rec_layer_s3.c +@@ -230,6 +230,12 @@ int ssl3_read_n(SSL *s, size_t n, size_t max, int extend, int clearold, + /* ... now we can act as if 'extend' was set */ + } + ++ if (!ossl_assert(s->rlayer.packet != NULL)) { ++ /* does not happen */ ++ SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); ++ return -1; ++ } ++ + len = s->rlayer.packet_length; + pkt = rb->buf + align; + /* +diff --git a/ssl/record/ssl3_buffer.c b/ssl/record/ssl3_buffer.c +index 97b0c26..1a10a7c 100644 +--- a/ssl/record/ssl3_buffer.c ++++ b/ssl/record/ssl3_buffer.c +@@ -191,5 +191,7 @@ int ssl3_release_read_buffer(SSL *s) + OPENSSL_cleanse(b->buf, b->len); + OPENSSL_free(b->buf); + b->buf = NULL; ++ s->rlayer.packet = NULL; ++ s->rlayer.packet_length = 0; + return 1; + } +-- +2.33.0 + diff --git a/backport-CVE-2024-4741-fixup-Further-extend-the-SSL_-6.patch b/backport-CVE-2024-4741-fixup-Further-extend-the-SSL_-6.patch new file mode 100644 index 0000000000000000000000000000000000000000..29be8f1dde7873466cc2501c16de77182a1b18c9 --- /dev/null +++ b/backport-CVE-2024-4741-fixup-Further-extend-the-SSL_-6.patch @@ -0,0 +1,34 @@ +From 8e40d82b65d82b7187d94be4e96b871178ea03fa Mon Sep 17 00:00:00 2001 +From: Matt Caswell +Date: Wed, 29 May 2024 10:17:58 +0800 +Subject: [PATCH] fixup! Further extend the SSL_free_buffers testing + +--- + test/sslbuffertest.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/test/sslbuffertest.c b/test/sslbuffertest.c +index aad770c..c3bed93 100644 +--- a/test/sslbuffertest.c ++++ b/test/sslbuffertest.c +@@ -170,7 +170,7 @@ static int test_free_buffers(int test) + const char testdata[] = "Test data"; + char buf[120]; + size_t written, readbytes; +- int pipeline = test > 3; ++ int i, pipeline = test > 3; + ENGINE *e = NULL; + + if (pipeline) { +@@ -200,7 +200,7 @@ static int test_free_buffers(int test) + * For the non-pipeline case we write one record. For pipelining we write + * two records. + */ +- for (int i = 0; i <= pipeline; i++) { ++ for (i = 0; i <= pipeline; i++) { + if (!TEST_true(SSL_write_ex(clientssl, testdata, strlen(testdata), + &written))) + goto end; +-- +2.33.0 + diff --git a/backport-CVE-2024-4741-fixup-Further-extend-the-SSL_-7.patch b/backport-CVE-2024-4741-fixup-Further-extend-the-SSL_-7.patch new file mode 100644 index 0000000000000000000000000000000000000000..33c2c39f9778c378c879a71f01acd28c623a6beb --- /dev/null +++ b/backport-CVE-2024-4741-fixup-Further-extend-the-SSL_-7.patch @@ -0,0 +1,50 @@ +From 37defd3ba2f2f5182b698d2aad44092b093217b8 Mon Sep 17 00:00:00 2001 +From: Matt Caswell +Date: Wed, 29 May 2024 10:21:57 +0800 +Subject: [PATCH] fixup! Further extend the SSL_free_buffers testing + +--- + test/sslbuffertest.c | 16 ++++++++++++++++ + 1 file changed, 16 insertions(+) + +diff --git a/test/sslbuffertest.c b/test/sslbuffertest.c +index c3bed93..e937ca2 100644 +--- a/test/sslbuffertest.c ++++ b/test/sslbuffertest.c +@@ -8,10 +8,19 @@ + * or in the file LICENSE in the source distribution. + */ + ++/* ++ * We need access to the deprecated low level Engine APIs for legacy purposes ++ * when the deprecated calls are not hidden ++ */ ++#ifndef OPENSSL_NO_DEPRECATED_3_0 ++# define OPENSSL_SUPPRESS_DEPRECATED ++#endif ++ + #include + #include + #include + #include ++#include + + #include "internal/packet.h" + +@@ -301,6 +310,13 @@ static int test_free_buffers(int test) + SSL_free(clientssl); + SSL_free(serverssl); + ++#ifndef OPENSSL_NO_DYNAMIC_ENGINE ++ if (e != NULL) { ++ ENGINE_unregister_ciphers(e); ++ ENGINE_finish(e); ++ ENGINE_free(e); ++ } ++#endif + return result; + } + +-- +2.33.0 + diff --git a/openssl.spec b/openssl.spec index 6024be9e945390dea493f9cb7dd12539514e7784..b6d8debff56d7ee28b4c4248309ce6c2698f0237 100644 --- a/openssl.spec +++ b/openssl.spec @@ -2,7 +2,7 @@ Name: openssl Epoch: 1 Version: 3.0.12 -Release: 5 +Release: 6 Summary: Cryptography and SSL/TLS Toolkit License: OpenSSL and SSLeay URL: https://www.openssl.org/ @@ -38,6 +38,13 @@ Patch26: backport-Extend-the-multi_resume-test-for-simultaneous-resump.patch Patch27: backport-Hardening-around-not_resumable-sessions.patch Patch28: backport-Add-a-test-for-session-cache-overflow.patch Patch29: backport-CVE-2024-4603-Check-DSA-parameters-for-exce.patch +Patch30: backport-CVE-2024-4741-Only-free-the-read-buffers-if-1.patch +Patch31: backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-aft-2.patch +Patch32: backport-CVE-2024-4741-Extend-the-SSL_free_buffers-t-3.patch +Patch33: backport-CVE-2024-4741-Move-the-ability-to-load-the-4.patch +Patch34: backport-CVE-2024-4741-Further-extend-the-SSL_free_b-5.patch +Patch35: backport-CVE-2024-4741-fixup-Further-extend-the-SSL_-6.patch +Patch36: backport-CVE-2024-4741-fixup-Further-extend-the-SSL_-7.patch BuildRequires: gcc gcc-c++ perl make lksctp-tools-devel coreutils util-linux zlib-devel Requires: coreutils %{name}-libs%{?_isa} = %{epoch}:%{version}-%{release} @@ -238,6 +245,9 @@ make test || : %ldconfig_scriptlets libs %changelog +* Wed May 29 2024 cenhuilin - 1:3.0.12-6 +- fix CVE-2024-4741 + * Fri May 17 2024 cenhuilin - 1:3.0.12-5 - fix CVE-2024-4603