diff --git a/Backport-CVE-2024-4741-Extend-the-SSL_free_buffers-testing.patch b/Backport-CVE-2024-4741-Extend-the-SSL_free_buffers-testing.patch new file mode 100644 index 0000000000000000000000000000000000000000..752625ad32a71e168b290bfa7d32e0c6c11382c7 --- /dev/null +++ b/Backport-CVE-2024-4741-Extend-the-SSL_free_buffers-testing.patch @@ -0,0 +1,133 @@ +From 6fef334f914abfcd988e53a32d19f01d84529f74 Mon Sep 17 00:00:00 2001 +From: Matt Caswell +Date: Thu, 25 Apr 2024 09:34:16 +0100 +Subject: [PATCH 3/5] 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 + +Reviewed-by: Tomas Mraz +Reviewed-by: Neil Horman +(Merged from https://github.com/openssl/openssl/pull/24395) + +(cherry picked from commit 4238abc17d44383592f92d6254d89dac806ee76b) +--- + test/sslbuffertest.c | 93 ++++++++++++++++++++++++++++++++++++++++++++ + 1 file changed, 93 insertions(+) + +diff --git a/test/sslbuffertest.c b/test/sslbuffertest.c +index 3c3e69d61d..133fdb13ee 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_buffers-testing.patch b/Backport-CVE-2024-4741-Further-extend-the-SSL_free_buffers-testing.patch new file mode 100644 index 0000000000000000000000000000000000000000..e8d186c26e9eda73ab9ecfdf3c174b8cca19af13 --- /dev/null +++ b/Backport-CVE-2024-4741-Further-extend-the-SSL_free_buffers-testing.patch @@ -0,0 +1,201 @@ +From d095674320c84b8ed1250715b1dd5ce05f9f267b Mon Sep 17 00:00:00 2001 +From: Matt Caswell +Date: Fri, 26 Apr 2024 13:58:29 +0100 +Subject: [PATCH 5/5] 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 + +Reviewed-by: Tomas Mraz +Reviewed-by: Neil Horman +(Merged from https://github.com/openssl/openssl/pull/24395) + +(cherry picked from commit 6972d5ace1275faf404e7a53e806861962f4121c) +--- + test/sslbuffertest.c | 113 +++++++++++++++++++++++++++++++++++++------ + 1 file changed, 97 insertions(+), 16 deletions(-) + +diff --git a/test/sslbuffertest.c b/test/sslbuffertest.c +index 133fdb13ee..7079d04e15 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" + +@@ -160,34 +169,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 i, 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 (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 +255,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; ++ } + } + + /* +@@ -238,7 +309,13 @@ static int test_free_buffers(int test) + end: + 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; + } + +@@ -265,7 +342,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-dasync-engine-into-sslt.patch b/Backport-CVE-2024-4741-Move-the-ability-to-load-the-dasync-engine-into-sslt.patch new file mode 100644 index 0000000000000000000000000000000000000000..2805821b801705211ec61280f20b3d1812e99c6d --- /dev/null +++ b/Backport-CVE-2024-4741-Move-the-ability-to-load-the-dasync-engine-into-sslt.patch @@ -0,0 +1,87 @@ +From 1359c00e683840154760b7ba9204bad1b13dc074 Mon Sep 17 00:00:00 2001 +From: Matt Caswell +Date: Fri, 26 Apr 2024 11:05:52 +0100 +Subject: [PATCH 4/5] 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 + +Reviewed-by: Tomas Mraz +Reviewed-by: Neil Horman +(Merged from https://github.com/openssl/openssl/pull/24395) + +(cherry picked from commit 0544c21a22f4d787e6f31d35e8f980402ac90a6d) +--- + test/helpers/ssltestlib.c | 33 +++++++++++++++++++++++++++++++++ + test/helpers/ssltestlib.h | 1 + + test/sslapitest.c | 21 --------------------- + 3 files changed, 34 insertions(+), 21 deletions(-) + +diff --git a/test/helpers/ssltestlib.c b/test/helpers/ssltestlib.c +index ef4a6177aa..da14f6697d 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 8e9daa5601..2777fb3047 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-we-re-not-using-them.patch b/Backport-CVE-2024-4741-Only-free-the-read-buffers-if-we-re-not-using-them.patch new file mode 100644 index 0000000000000000000000000000000000000000..596b871a64666caac54e02d076d8fd1aaa69f659 --- /dev/null +++ b/Backport-CVE-2024-4741-Only-free-the-read-buffers-if-we-re-not-using-them.patch @@ -0,0 +1,72 @@ +From b3f0eb0a295f58f16ba43ba99dad70d4ee5c437d Mon Sep 17 00:00:00 2001 +From: Watson Ladd +Date: Wed, 24 Apr 2024 11:26:56 +0100 +Subject: [PATCH 1/5] 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 + +Reviewed-by: Tomas Mraz +Reviewed-by: Neil Horman +Reviewed-by: Matt Caswell +(Merged from https://github.com/openssl/openssl/pull/24395) + +(cherry picked from commit 704f725b96aa373ee45ecfb23f6abfe8be8d9177) +--- + 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 4bcffcc41e..1569997bea 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 234656bf93..b60f71c8cb 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 eed649c6fd..d14c55ae55 100644 +--- a/ssl/ssl_lib.c ++++ b/ssl/ssl_lib.c +@@ -5492,6 +5492,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-after-we-ve-finished-using.patch b/Backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-after-we-ve-finished-using.patch new file mode 100644 index 0000000000000000000000000000000000000000..3514fc8ddc815c3a587e4228136d18f777bc8041 --- /dev/null +++ b/Backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-after-we-ve-finished-using.patch @@ -0,0 +1,52 @@ +From 2d05959073c4bf8803401668b9df85931a08e020 Mon Sep 17 00:00:00 2001 +From: Matt Caswell +Date: Wed, 24 Apr 2024 11:33:41 +0100 +Subject: [PATCH 2/5] 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 + +Reviewed-by: Tomas Mraz +Reviewed-by: Neil Horman +(Merged from https://github.com/openssl/openssl/pull/24395) + +(cherry picked from commit d146349171101dec3a876c13eb7a6dea32ba62ba) +--- + 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 1569997bea..779e998bb6 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 97b0c26ced..1a10a7c0b8 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/openssl.spec b/openssl.spec index 0041cc54a249141355aa29084735cd50f654f8ac..bb40a3282476c6a85448cc7068d134a9c53d610c 100644 --- a/openssl.spec +++ b/openssl.spec @@ -4,7 +4,7 @@ Name: openssl Epoch: 1 Version: 3.0.9 -Release: 5 +Release: 6 Summary: Cryptography and SSL/TLS Toolkit License: OpenSSL and SSLeay URL: https://www.openssl.org/ @@ -35,6 +35,11 @@ Patch21: backport-dhtest.c-Add-test-of-DH_check-with-q-p-1.patch Patch22: Backport-support-decode-SM2-parameters.patch Patch23: Feature-support-SM2-CMS-signature.patch Patch24: Feature-use-default-id-if-SM2-id-is-not-set.patch +Patch25: Backport-CVE-2024-4741-Only-free-the-read-buffers-if-we-re-not-using-them.patch +Patch26: Backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-after-we-ve-finished-using.patch +Patch27: Backport-CVE-2024-4741-Extend-the-SSL_free_buffers-testing.patch +Patch28: Backport-CVE-2024-4741-Move-the-ability-to-load-the-dasync-engine-into-sslt.patch +Patch29: Backport-CVE-2024-4741-Further-extend-the-SSL_free_buffers-testing.patch BuildRequires: gcc gcc-c++ perl make lksctp-tools-devel coreutils util-linux zlib-devel Requires: coreutils %{name}-libs%{?_isa} = %{epoch}:%{version}-%{release} @@ -261,6 +266,9 @@ make test || : %endif %changelog +* Thu Jun 20 2024 steven - 1:3.0.9-6 +- fix CVE-2024-4741 + * Wed Sep 13 2023 taoyuxiang - 1:3.0.9-5 - add sub rpm openssl-relocation