1From fc43b2b1abae58c1b261962299d2bbeee770810a Mon Sep 17 00:00:00 2001 2From: jxlang910 <jinxiulang@huawei.com> 3Date: Thu, 11 Apr 2024 17:24:44 +0800 4Subject: [PATCH] fix CVE-2024-2511 5 6--- 7 include/openssl/sslerr.h | 4 +- 8 ssl/ssl_err.c | 5 +- 9 ssl/ssl_lib.c | 5 +- 10 ssl/ssl_sess.c | 36 ++++- 11 ssl/statem/statem_srvr.c | 5 +- 12 test/sslapitest.c | 300 +++++++++++++++++++++++++++++++++++++++ 13 6 files changed, 339 insertions(+), 16 deletions(-) 14 15diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h 16index aa5f56a482..3e99ffc27f 100644 17--- a/include/openssl/sslerr.h 18+++ b/include/openssl/sslerr.h 19@@ -1,6 +1,6 @@ 20 /* 21 * Generated by util/mkerr.pl DO NOT EDIT 22- * Copyright 1995-2020 The OpenSSL Project Authors. All Rights Reserved. 23+ * Copyright 1995-2024 The OpenSSL Project Authors. All Rights Reserved. 24 * 25 * Licensed under the OpenSSL license (the "License"). You may not use 26 * this file except in compliance with the License. You can obtain a copy 27@@ -224,7 +224,7 @@ int ERR_load_SSL_strings(void); 28 # define SSL_F_SSL_RENEGOTIATE_ABBREVIATED 546 29 # define SSL_F_SSL_SCAN_CLIENTHELLO_TLSEXT 320 30 # define SSL_F_SSL_SCAN_SERVERHELLO_TLSEXT 321 31-# define SSL_F_SSL_SESSION_DUP 348 32+# define SSL_F_SSL_SESSION_DUP_INTERN 668 33 # define SSL_F_SSL_SESSION_NEW 189 34 # define SSL_F_SSL_SESSION_PRINT_FP 190 35 # define SSL_F_SSL_SESSION_SET1_ID 423 36diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c 37index 5a7c42a88c..c4144bb8b4 100644 38--- a/ssl/ssl_err.c 39+++ b/ssl/ssl_err.c 40@@ -1,6 +1,6 @@ 41 /* 42 * Generated by util/mkerr.pl DO NOT EDIT 43- * Copyright 1995-2019 The OpenSSL Project Authors. All Rights Reserved. 44+ * Copyright 1995-2024 The OpenSSL Project Authors. All Rights Reserved. 45 * 46 * Licensed under the OpenSSL license (the "License"). You may not use 47 * this file except in compliance with the License. You can obtain a copy 48@@ -325,7 +325,8 @@ static const ERR_STRING_DATA SSL_str_functs[] = { 49 "SSL_renegotiate_abbreviated"}, 50 {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SCAN_CLIENTHELLO_TLSEXT, 0), ""}, 51 {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SCAN_SERVERHELLO_TLSEXT, 0), ""}, 52- {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_DUP, 0), "ssl_session_dup"}, 53+ {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_DUP_INTERN, 0), 54+ "ssl_session_dup_intern"}, 55 {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_NEW, 0), "SSL_SESSION_new"}, 56 {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_PRINT_FP, 0), 57 "SSL_SESSION_print_fp"}, 58diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c 59index 618549a2ca..2a44960fac 100644 60--- a/ssl/ssl_lib.c 61+++ b/ssl/ssl_lib.c 62@@ -3541,9 +3541,10 @@ void ssl_update_cache(SSL *s, int mode) 63 64 /* 65 * If the session_id_length is 0, we are not supposed to cache it, and it 66- * would be rather hard to do anyway :-) 67+ * would be rather hard to do anyway :-). Also if the session has already 68+ * been marked as not_resumable we should not cache it for later reuse. 69 */ 70- if (s->session->session_id_length == 0) 71+ if (s->session->session_id_length == 0 || s->session->not_resumable) 72 return; 73 74 /* 75diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c 76index 1b4c85b60c..5cc816b0fc 100644 77--- a/ssl/ssl_sess.c 78+++ b/ssl/ssl_sess.c 79@@ -94,16 +94,11 @@ SSL_SESSION *SSL_SESSION_new(void) 80 return ss; 81 } 82 83-SSL_SESSION *SSL_SESSION_dup(SSL_SESSION *src) 84-{ 85- return ssl_session_dup(src, 1); 86-} 87- 88 /* 89 * Create a new SSL_SESSION and duplicate the contents of |src| into it. If 90 * ticket == 0 then no ticket information is duplicated, otherwise it is. 91 */ 92-SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket) 93+static SSL_SESSION *ssl_session_dup_intern(SSL_SESSION *src, int ticket) 94 { 95 SSL_SESSION *dest; 96 97@@ -221,11 +216,32 @@ SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket) 98 99 return dest; 100 err: 101- SSLerr(SSL_F_SSL_SESSION_DUP, ERR_R_MALLOC_FAILURE); 102+ SSLerr(SSL_F_SSL_SESSION_DUP_INTERN, ERR_R_MALLOC_FAILURE); 103 SSL_SESSION_free(dest); 104 return NULL; 105 } 106 107+SSL_SESSION *SSL_SESSION_dup(SSL_SESSION *src) 108+{ 109+ return ssl_session_dup_intern(src, 1); 110+} 111+ 112+/* 113+ * Used internally when duplicating a session which might be already shared. 114+ * We will have resumed the original session. Subsequently we might have marked 115+ * it as non-resumable (e.g. in another thread) - but this copy should be ok to 116+ * resume from. 117+ */ 118+SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket) 119+{ 120+ SSL_SESSION *sess = ssl_session_dup_intern(src, ticket); 121+ 122+ if (sess != NULL) 123+ sess->not_resumable = 0; 124+ 125+ return sess; 126+} 127+ 128 const unsigned char *SSL_SESSION_get_id(const SSL_SESSION *s, unsigned int *len) 129 { 130 if (len) 131@@ -455,6 +471,12 @@ SSL_SESSION *lookup_sess_in_cache(SSL *s, const unsigned char *sess_id, 132 ret = s->session_ctx->get_session_cb(s, sess_id, sess_id_len, ©); 133 134 if (ret != NULL) { 135+ if (ret->not_resumable) { 136+ /* If its not resumable then ignore this session */ 137+ if (!copy) 138+ SSL_SESSION_free(ret); 139+ return NULL; 140+ } 141 tsan_counter(&s->session_ctx->stats.sess_cb_hit); 142 143 /* 144diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c 145index 1b3b8002ee..d242e98024 100644 146--- a/ssl/statem/statem_srvr.c 147+++ b/ssl/statem/statem_srvr.c 148@@ -2418,9 +2418,8 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt) 149 * so the following won't overwrite an ID that we're supposed 150 * to send back. 151 */ 152- if (s->session->not_resumable || 153- (!(s->ctx->session_cache_mode & SSL_SESS_CACHE_SERVER) 154- && !s->hit)) 155+ if (!(s->ctx->session_cache_mode & SSL_SESS_CACHE_SERVER) 156+ && !s->hit) 157 s->session->session_id_length = 0; 158 159 if (usetls13) { 160diff --git a/test/sslapitest.c b/test/sslapitest.c 161index 5ee982ab06..395b1e5457 100644 162--- a/test/sslapitest.c 163+++ b/test/sslapitest.c 164@@ -6669,6 +6669,128 @@ static int test_ca_names(int tst) 165 return testresult; 166 } 167 168+/* 169+ * Test that a session cache overflow works as expected 170+ * Test 0: TLSv1.3, timeout on new session later than old session 171+ * Test 1: TLSv1.2, timeout on new session later than old session 172+ * Test 2: TLSv1.3, timeout on new session earlier than old session 173+ * Test 3: TLSv1.2, timeout on new session earlier than old session 174+ */ 175+#if !defined(OPENSSL_NO_TLS1_3) || !defined(OPENSSL_NO_TLS1_2) 176+static int test_session_cache_overflow(int idx) 177+{ 178+ SSL_CTX *sctx = NULL, *cctx = NULL; 179+ SSL *serverssl = NULL, *clientssl = NULL; 180+ int testresult = 0; 181+ SSL_SESSION *sess = NULL; 182+ 183+#ifdef OPENSSL_NO_TLS1_3 184+ /* If no TLSv1.3 available then do nothing in this case */ 185+ if (idx % 2 == 0) 186+ TEST_info("No TLSv1.3 available"); 187+ return 1; 188+#endif 189+#ifdef OPENSSL_NO_TLS1_2 190+ /* If no TLSv1.2 available then do nothing in this case */ 191+ if (idx % 2 == 1) 192+ TEST_info("No TLSv1.2 available"); 193+ return 1; 194+#endif 195+ 196+ if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(), 197+ TLS_client_method(), TLS1_VERSION, 198+ (idx % 2 == 0) ? TLS1_3_VERSION 199+ : TLS1_2_VERSION, 200+ &sctx, &cctx, cert, privkey)) 201+ || !TEST_true(SSL_CTX_set_options(sctx, SSL_OP_NO_TICKET))) 202+ goto end; 203+ 204+ SSL_CTX_sess_set_get_cb(sctx, get_session_cb); 205+ get_sess_val = NULL; 206+ 207+ SSL_CTX_sess_set_cache_size(sctx, 1); 208+ 209+ if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, 210+ NULL, NULL))) 211+ goto end; 212+ 213+ if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))) 214+ goto end; 215+ 216+ if (idx > 1) { 217+ sess = SSL_get_session(serverssl); 218+ if (!TEST_ptr(sess)) 219+ goto end; 220+ 221+ /* 222+ * Cause this session to have a longer timeout than the next session to 223+ * be added. 224+ */ 225+ if (!TEST_true(SSL_SESSION_set_timeout(sess, LONG_MAX / 2))) { 226+ sess = NULL; 227+ goto end; 228+ } 229+ sess = NULL; 230+ } 231+ 232+ SSL_shutdown(serverssl); 233+ SSL_shutdown(clientssl); 234+ SSL_free(serverssl); 235+ SSL_free(clientssl); 236+ serverssl = clientssl = NULL; 237+ 238+ /* 239+ * Session cache size is 1 and we already populated the cache with a session 240+ * so the next connection should cause an overflow. 241+ */ 242+ 243+ if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, 244+ NULL, NULL))) 245+ goto end; 246+ 247+ if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))) 248+ goto end; 249+ 250+ /* 251+ * The session we just negotiated may have been already removed from the 252+ * internal cache - but we will return it anyway from our external cache. 253+ */ 254+ get_sess_val = SSL_get_session(serverssl); 255+ if (!TEST_ptr(get_sess_val)) 256+ goto end; 257+ sess = SSL_get1_session(clientssl); 258+ if (!TEST_ptr(sess)) 259+ goto end; 260+ 261+ SSL_shutdown(serverssl); 262+ SSL_shutdown(clientssl); 263+ SSL_free(serverssl); 264+ SSL_free(clientssl); 265+ serverssl = clientssl = NULL; 266+ 267+ if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, 268+ NULL, NULL))) 269+ goto end; 270+ 271+ if (!TEST_true(SSL_set_session(clientssl, sess))) 272+ goto end; 273+ 274+ if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))) 275+ goto end; 276+ 277+ testresult = 1; 278+ 279+ end: 280+ SSL_free(serverssl); 281+ SSL_free(clientssl); 282+ SSL_CTX_free(sctx); 283+ SSL_CTX_free(cctx); 284+ SSL_SESSION_free(sess); 285+ 286+ return testresult; 287+} 288+#endif /* !defined(OPENSSL_NO_TLS1_3) || !defined(OPENSSL_NO_TLS1_2) */ 289+ 290 /* 291 * Test 0: Client sets servername and server acknowledges it (TLSv1.2) 292 * Test 1: Client sets servername and server does not acknowledge it (TLSv1.2) 293@@ -7288,6 +7410,180 @@ static int test_inherit_verify_param(void) 294 return testresult; 295 } 296 297+struct resume_servername_cb_data { 298+ int i; 299+ SSL_CTX *cctx; 300+ SSL_CTX *sctx; 301+ SSL_SESSION *sess; 302+ int recurse; 303+}; 304+ 305+/* 306+ * Servername callback. We use it here to run another complete handshake using 307+ * the same session - and mark the session as not_resuamble at the end 308+ */ 309+static int resume_servername_cb(SSL *s, int *ad, void *arg) 310+{ 311+ struct resume_servername_cb_data *cbdata = arg; 312+ SSL *serverssl = NULL, *clientssl = NULL; 313+ int ret = SSL_TLSEXT_ERR_ALERT_FATAL; 314+ 315+ if (cbdata->recurse) 316+ return SSL_TLSEXT_ERR_ALERT_FATAL; 317+ 318+ if ((cbdata->i % 3) != 1) 319+ return SSL_TLSEXT_ERR_OK; 320+ 321+ cbdata->recurse = 1; 322+ 323+ if (!TEST_true(create_ssl_objects(cbdata->sctx, cbdata->cctx, &serverssl, 324+ &clientssl, NULL, NULL)) 325+ || !TEST_true(SSL_set_session(clientssl, cbdata->sess))) 326+ goto end; 327+ 328+ ERR_set_mark(); 329+ /* 330+ * We expect this to fail - because the servername cb will fail. This will 331+ * mark the session as not_resumable. 332+ */ 333+ if (!TEST_false(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))) { 334+ ERR_clear_last_mark(); 335+ goto end; 336+ } 337+ ERR_pop_to_mark(); 338+ 339+ ret = SSL_TLSEXT_ERR_OK; 340+ end: 341+ SSL_free(serverssl); 342+ SSL_free(clientssl); 343+ cbdata->recurse = 0; 344+ return ret; 345+} 346+ 347+/* 348+ * Test multiple resumptions and cache size handling 349+ * Test 0: TLSv1.3 (max_early_data set) 350+ * Test 1: TLSv1.3 (SSL_OP_NO_TICKET set) 351+ * Test 2: TLSv1.3 (max_early_data and SSL_OP_NO_TICKET set) 352+ * Test 3: TLSv1.3 (SSL_OP_NO_TICKET, simultaneous resumes) 353+ * Test 4: TLSv1.2 354+ */ 355+static int test_multi_resume(int idx) 356+{ 357+ SSL_CTX *sctx = NULL, *cctx = NULL; 358+ SSL *serverssl = NULL, *clientssl = NULL; 359+ SSL_SESSION *sess = NULL; 360+ int max_version = TLS1_3_VERSION; 361+ int i, testresult = 0; 362+ struct resume_servername_cb_data cbdata; 363+ 364+#if defined(OPENSSL_NO_TLS1_2) 365+ if (idx == 4) 366+ TEST_info("TLSv1.2 is disabled in this build"); 367+ return 1; 368+#else 369+ if (idx == 4) 370+ max_version = TLS1_2_VERSION; 371+#endif 372+#if defined(OPENSSL_NO_TLS1_3) 373+ if (idx != 4) 374+ TEST_info("No usable TLSv1.3 in this build"); 375+ return 1; 376+#endif 377+ 378+ if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(), 379+ TLS_client_method(), TLS1_VERSION, 380+ max_version, &sctx, &cctx, cert, 381+ privkey))) 382+ goto end; 383+ 384+ /* 385+ * TLSv1.3 only uses a session cache if either max_early_data > 0 (used for 386+ * replay protection), or if SSL_OP_NO_TICKET is in use 387+ */ 388+ if (idx == 0 || idx == 2) { 389+ if (!TEST_true(SSL_CTX_set_max_early_data(sctx, 1024))) 390+ goto end; 391+ } 392+ if (idx == 1 || idx == 2 || idx == 3) 393+ SSL_CTX_set_options(sctx, SSL_OP_NO_TICKET); 394+ 395+ SSL_CTX_sess_set_cache_size(sctx, 5); 396+ 397+ if (idx == 3) { 398+ SSL_CTX_set_tlsext_servername_callback(sctx, resume_servername_cb); 399+ SSL_CTX_set_tlsext_servername_arg(sctx, &cbdata); 400+ cbdata.cctx = cctx; 401+ cbdata.sctx = sctx; 402+ cbdata.recurse = 0; 403+ } 404+ 405+ for (i = 0; i < 30; i++) { 406+ if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, 407+ NULL, NULL)) 408+ || !TEST_true(SSL_set_session(clientssl, sess))) 409+ goto end; 410+ 411+ /* 412+ * Check simultaneous resumes. We pause the connection part way through 413+ * the handshake by (mis)using the servername_cb. The pause occurs after 414+ * session resumption has already occurred, but before any session 415+ * tickets have been issued. While paused we run another complete 416+ * handshake resuming the same session. 417+ */ 418+ if (idx == 3) { 419+ cbdata.i = i; 420+ cbdata.sess = sess; 421+ } 422+ 423+ /* 424+ * Recreate a bug where dynamically changing the max_early_data value 425+ * can cause sessions in the session cache which cannot be deleted. 426+ */ 427+ if ((idx == 0 || idx == 2) && (i % 3) == 2) 428+ SSL_set_max_early_data(serverssl, 0); 429+ 430+ if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))) 431+ goto end; 432+ 433+ if (sess == NULL || (idx == 0 && (i % 3) == 2)) { 434+ if (!TEST_false(SSL_session_reused(clientssl))) 435+ goto end; 436+ } else { 437+ if (!TEST_true(SSL_session_reused(clientssl))) 438+ goto end; 439+ } 440+ SSL_SESSION_free(sess); 441+ 442+ /* Do a full handshake, followed by two resumptions */ 443+ if ((i % 3) == 2) { 444+ sess = NULL; 445+ } else { 446+ if (!TEST_ptr((sess = SSL_get1_session(clientssl)))) 447+ goto end; 448+ } 449+ 450+ SSL_shutdown(clientssl); 451+ SSL_shutdown(serverssl); 452+ SSL_free(serverssl); 453+ SSL_free(clientssl); 454+ serverssl = clientssl = NULL; 455+ } 456+ 457+ /* We should never exceed the session cache size limit */ 458+ if (!TEST_long_le(SSL_CTX_sess_number(sctx), 5)) 459+ goto end; 460+ 461+ testresult = 1; 462+ end: 463+ SSL_free(serverssl); 464+ SSL_free(clientssl); 465+ SSL_CTX_free(sctx); 466+ SSL_CTX_free(cctx); 467+ SSL_SESSION_free(sess); 468+ return testresult; 469+} 470+ 471 int setup_tests(void) 472 { 473 if (!TEST_ptr(certsdir = test_get_argument(0)) 474@@ -7422,6 +7718,10 @@ int setup_tests(void) 475 #if !defined(OPENSSL_NO_TLS1_2) && !defined(OPENSSL_NO_TLS1_3) 476 ADD_ALL_TESTS(test_serverinfo_custom, 4); 477 #endif 478+#if !defined(OPENSSL_NO_TLS1_2) || !defined(OPENSSL_NO_TLS1_3) 479+ ADD_ALL_TESTS(test_session_cache_overflow, 4); 480+#endif 481+ ADD_ALL_TESTS(test_multi_resume, 5); 482 return 1; 483 } 484 485-- 4862.43.0.windows.1 487 488