packages/openssl
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix CVE-2024-4741

Browse Source 
(cherry picked from commit 222387cff33f323c0bcf789ccca08e98bdb22b70)
This commit is contained in:
hzero1996 2024-06-03 16:10:14 +08:00 committed by openeuler-sync-bot
parent bffccbf08a
commit 8cbd3c76fb
6 changed files with 554 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

133
Backport-CVE-2024-4741-Extend-the-SSL_free_buffers-testing.patch Normal file
View File

@ -0,0 +1,133 @@
From 6fef334f914abfcd988e53a32d19f01d84529f74 Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.org>
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 <tomas@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
(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

201
Backport-CVE-2024-4741-Further-extend-the-SSL_free_buffers-testing.patch Normal file
View File

@ -0,0 +1,201 @@
From d095674320c84b8ed1250715b1dd5ce05f9f267b Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.org>
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 <tomas@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
(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 <string.h>
#include <openssl/ssl.h>
#include <openssl/bio.h>
#include <openssl/err.h>
+#include <openssl/engine.h>
#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

87
Backport-CVE-2024-4741-Move-the-ability-to-load-the-dasync-engine-into-sslt.patch Normal file
View File

@ -0,0 +1,87 @@
From 1359c00e683840154760b7ba9204bad1b13dc074 Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.org>
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 <tomas@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
(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 <string.h>
+#include <openssl/engine.h>
#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

72
Backport-CVE-2024-4741-Only-free-the-read-buffers-if-we-re-not-using-them.patch Normal file
View File

@ -0,0 +1,72 @@
From b3f0eb0a295f58f16ba43ba99dad70d4ee5c437d Mon Sep 17 00:00:00 2001
From: Watson Ladd <watsonbladd@gmail.com>
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 <tomas@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(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

52
Backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-after-we-ve-finished-using.patch Normal file
View File

@ -0,0 +1,52 @@
From 2d05959073c4bf8803401668b9df85931a08e020 Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.org>
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 <tomas@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
(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

10
openssl.spec
View File

@ -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,11 @@ 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-we-re-not-using-them.patch
Patch31: Backport-CVE-2024-4741-Set-rlayer.packet-to-NULL-after-we-ve-finished-using.patch
Patch32: Backport-CVE-2024-4741-Extend-the-SSL_free_buffers-testing.patch
Patch33: Backport-CVE-2024-4741-Move-the-ability-to-load-the-dasync-engine-into-sslt.patch
Patch34: 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}
@ -238,6 +243,9 @@ make test || :
%ldconfig_scriptlets libs
%changelog
* Mon Jun 3 2024 wangcheng <wangcheng156@huawei.com> - 1:3.0.12-6
- fix CVE-2024-4741
* Fri May 17 2024 cenhuilin <cenhuilin@kylinos.cn> - 1:3.0.12-5
- fix CVE-2024-4603