diff --git a/CHANGES b/CHANGES index c815457e2..070e438aa 100644 --- a/CHANGES +++ b/CHANGES @@ -7,6 +7,8 @@ Changes between 8.3.3 and 8.3.4 [xxxx年xx月xx日] + *) 修复CVE-2024-2511 + *) 修复CVE-2024-0727 *) 修复CVE-2023-4807 diff --git a/CHANGES.en b/CHANGES.en index b1e869e71..846f28c39 100644 --- a/CHANGES.en +++ b/CHANGES.en @@ -7,6 +7,8 @@ Changes between 8.3.3 and 8.3.4 [xx XXX xxxx] + *) Fix CVE-2024-2511 + *) Fix CVE-2024-0727 *) Fix CVE-2023-4807 diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index bdf636b2c..1f9ee3ac7 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -1466,7 +1466,7 @@ SSL_F_SSL_RENEGOTIATE:516:SSL_renegotiate SSL_F_SSL_RENEGOTIATE_ABBREVIATED:546:SSL_renegotiate_abbreviated SSL_F_SSL_SCAN_CLIENTHELLO_TLSEXT:320:* SSL_F_SSL_SCAN_SERVERHELLO_TLSEXT:321:* -SSL_F_SSL_SESSION_DUP:348:ssl_session_dup +SSL_F_SSL_SESSION_DUP_INTERN:348:ssl_session_dup_intern SSL_F_SSL_SESSION_NEW:189:SSL_SESSION_new SSL_F_SSL_SESSION_PRINT_FP:190:SSL_SESSION_print_fp SSL_F_SSL_SESSION_SET1_ID:423:SSL_SESSION_set1_id diff --git a/doc/man3/SSL_SESSION_free.pod b/doc/man3/SSL_SESSION_free.pod index 9a3bf3ec9..48a9d37cd 100644 --- a/doc/man3/SSL_SESSION_free.pod +++ b/doc/man3/SSL_SESSION_free.pod @@ -12,7 +12,7 @@ SSL_SESSION_free - create, free and manage SSL_SESSION structures #include SSL_SESSION *SSL_SESSION_new(void); - SSL_SESSION *SSL_SESSION_dup(SSL_SESSION *src); + SSL_SESSION *SSL_SESSION_dup(const SSL_SESSION *src); int SSL_SESSION_up_ref(SSL_SESSION *ses); void SSL_SESSION_free(SSL_SESSION *session); diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index da3c4a1f7..69ed4b357 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -1763,7 +1763,7 @@ __owur int SSL_SESSION_set1_id(SSL_SESSION *s, const unsigned char *sid, __owur int SSL_SESSION_is_resumable(const SSL_SESSION *s); __owur SSL_SESSION *SSL_SESSION_new(void); -__owur SSL_SESSION *SSL_SESSION_dup(SSL_SESSION *src); +__owur SSL_SESSION *SSL_SESSION_dup(const SSL_SESSION *src); const unsigned char *SSL_SESSION_get_id(const SSL_SESSION *s, unsigned int *len); const unsigned char *SSL_SESSION_get0_id_context(const SSL_SESSION *s, diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h index 93bcec908..802d3341a 100644 --- a/include/openssl/sslerr.h +++ b/include/openssl/sslerr.h @@ -302,7 +302,7 @@ int ERR_load_SSL_strings(void); # define SSL_F_SSL_RENEGOTIATE_ABBREVIATED 546 # define SSL_F_SSL_SCAN_CLIENTHELLO_TLSEXT 320 # define SSL_F_SSL_SCAN_SERVERHELLO_TLSEXT 321 -# define SSL_F_SSL_SESSION_DUP 348 +# define SSL_F_SSL_SESSION_DUP_INTERN 348 # define SSL_F_SSL_SESSION_NEW 189 # define SSL_F_SSL_SESSION_PRINT_FP 190 # define SSL_F_SSL_SESSION_SET1_ID 423 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 6c2c82cf1..b1684e58f 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -477,7 +477,8 @@ static const ERR_STRING_DATA SSL_str_functs[] = { "SSL_renegotiate_abbreviated"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SCAN_CLIENTHELLO_TLSEXT, 0), ""}, {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SCAN_SERVERHELLO_TLSEXT, 0), ""}, - {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_DUP, 0), "ssl_session_dup"}, + {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_DUP_INTERN, 0), + "ssl_session_dup_intern"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_NEW, 0), "SSL_SESSION_new"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_PRINT_FP, 0), "SSL_SESSION_print_fp"}, diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 5205fec89..6168093aa 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -3616,9 +3616,10 @@ void ssl_update_cache(SSL *s, int mode) /* * If the session_id_length is 0, we are not supposed to cache it, and it - * would be rather hard to do anyway :-) + * would be rather hard to do anyway :-). Also if the session has already + * been marked as not_resumable we should not cache it for later reuse. */ - if (s->session->session_id_length == 0) + if (s->session->session_id_length == 0 || s->session->not_resumable) return; /* diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h index 5ea5e4aee..796c0a259 100644 --- a/ssl/ssl_local.h +++ b/ssl/ssl_local.h @@ -2498,7 +2498,7 @@ __owur int ssl_get_new_session(SSL *s, int session); __owur SSL_SESSION *lookup_sess_in_cache(SSL *s, const unsigned char *sess_id, size_t sess_id_len); __owur int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello); -__owur SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket); +__owur SSL_SESSION *ssl_session_dup(const SSL_SESSION *src, int ticket); __owur int ssl_cipher_id_cmp(const SSL_CIPHER *a, const SSL_CIPHER *b); DECLARE_OBJ_BSEARCH_GLOBAL_CMP_FN(SSL_CIPHER, SSL_CIPHER, ssl_cipher_id); __owur int ssl_cipher_ptr_id_cmp(const SSL_CIPHER *const *ap, diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 698be4884..517780a64 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -98,16 +98,11 @@ SSL_SESSION *SSL_SESSION_new(void) return ss; } -SSL_SESSION *SSL_SESSION_dup(SSL_SESSION *src) -{ - return ssl_session_dup(src, 1); -} - /* * Create a new SSL_SESSION and duplicate the contents of |src| into it. If * ticket == 0 then no ticket information is duplicated, otherwise it is. */ -SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket) +static SSL_SESSION *ssl_session_dup_intern(const SSL_SESSION *src, int ticket) { SSL_SESSION *dest; @@ -246,11 +241,32 @@ SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket) return dest; err: - SSLerr(SSL_F_SSL_SESSION_DUP, ERR_R_MALLOC_FAILURE); + SSLerr(SSL_F_SSL_SESSION_DUP_INTERN, ERR_R_MALLOC_FAILURE); SSL_SESSION_free(dest); return NULL; } +SSL_SESSION *SSL_SESSION_dup(const SSL_SESSION *src) +{ + return ssl_session_dup_intern(src, 1); +} + +/* + * Used internally when duplicating a session which might be already shared. + * We will have resumed the original session. Subsequently we might have marked + * it as non-resumable (e.g. in another thread) - but this copy should be ok to + * resume from. + */ +SSL_SESSION *ssl_session_dup(const SSL_SESSION *src, int ticket) +{ + SSL_SESSION *sess = ssl_session_dup_intern(src, ticket); + + if (sess != NULL) + sess->not_resumable = 0; + + return sess; +} + const unsigned char *SSL_SESSION_get_id(const SSL_SESSION *s, unsigned int *len) { if (len) @@ -501,6 +517,12 @@ SSL_SESSION *lookup_sess_in_cache(SSL *s, const unsigned char *sess_id, # endif if (ret != NULL) { + if (ret->not_resumable) { + /* If its not resumable then ignore this session */ + if (!copy) + SSL_SESSION_free(ret); + return NULL; + } tsan_counter(&s->session_ctx->stats.sess_cb_hit); /* @@ -774,27 +796,16 @@ int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *c) s = c; } - /* Put at the head of the queue unless it is already in the cache */ - if (s == NULL) - SSL_SESSION_list_add(ctx, c); - - if (s != NULL) { - /* - * existing cache entry -- decrement previously incremented reference - * count because it already takes into account the cache - */ - - SSL_SESSION_free(s); /* s == c */ - ret = 0; - } else { + if (s == NULL) { /* * new cache entry -- remove old ones if cache has become too large + * delete cache entry *before* add, so we don't remove the one we're adding! */ ret = 1; if (SSL_CTX_sess_get_cache_size(ctx) > 0) { - while (SSL_CTX_sess_number(ctx) > SSL_CTX_sess_get_cache_size(ctx)) { + while (SSL_CTX_sess_number(ctx) >= SSL_CTX_sess_get_cache_size(ctx)) { if (!remove_session_lock(ctx, ctx->session_cache_tail, 0)) break; else @@ -802,6 +813,18 @@ int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *c) } } } + + SSL_SESSION_list_add(ctx, c); + + if (s != NULL) { + /* + * existing cache entry -- decrement previously incremented reference + * count because it already takes into account the cache + */ + + SSL_SESSION_free(s); /* s == c */ + ret = 0; + } CRYPTO_THREAD_unlock(ctx->lock); return ret; } diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index d0aac9a64..a7a413584 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -2627,9 +2627,8 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt) * so the following won't overwrite an ID that we're supposed * to send back. */ - if (s->session->not_resumable || - (!(s->ctx->session_cache_mode & SSL_SESS_CACHE_SERVER) - && !s->hit)) + if (!(s->ctx->session_cache_mode & SSL_SESS_CACHE_SERVER) + && !s->hit) s->session->session_id_length = 0; if (usetls13) { diff --git a/test/sslapitest.c b/test/sslapitest.c index 98d4c7513..1a95a770b 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -1278,6 +1278,31 @@ static int execute_test_session(int maxprot, int use_int_cache, } } + /* + * Make a small cache, force out all other sessions but + * sess2, try to add sess1, which should succeed. Then + * make sure it's there by checking the owners. Despite + * the timeouts, sess1 should have kicked out sess2 + */ + + /* Make sess1 expire before sess2 */ + if (!TEST_long_gt(SSL_SESSION_set_time(sess1, 1000), 0) + || !TEST_long_gt(SSL_SESSION_set_timeout(sess1, 1000), 0) + || !TEST_long_gt(SSL_SESSION_set_time(sess2, 2000), 0) + || !TEST_long_gt(SSL_SESSION_set_timeout(sess2, 2000), 0)) + goto end; + + if (!TEST_long_ne(SSL_CTX_sess_set_cache_size(sctx, 1), 0)) + goto end; + + /* Don't care about results - cache should only be sess2 at end */ + SSL_CTX_add_session(sctx, sess1); + SSL_CTX_add_session(sctx, sess2); + + /* Now add sess1, and make sure it remains, despite timeout */ + if (!TEST_true(SSL_CTX_add_session(sctx, sess1))) + goto end; + testresult = 1; end: @@ -7138,6 +7163,297 @@ static int test_tls_cert_compression_api(int tst) } #endif +/* + * Test that a session cache overflow works as expected + * Test 0: TLSv1.3, timeout on new session later than old session + * Test 1: TLSv1.2, timeout on new session later than old session + * Test 2: TLSv1.3, timeout on new session earlier than old session + * Test 3: TLSv1.2, timeout on new session earlier than old session + */ +#if !defined(OPENSSL_NO_TLS1_3) || !defined(OPENSSL_NO_TLS1_2) +static int test_session_cache_overflow(int idx) +{ + SSL_CTX *sctx = NULL, *cctx = NULL; + SSL *serverssl = NULL, *clientssl = NULL; + int testresult = 0; + SSL_SESSION *sess = NULL; + +#ifdef OPENSSL_NO_TLS1_3 + /* If no TLSv1.3 available then do nothing in this case */ + if (idx % 2 == 0) + return 1; +#endif +#ifdef OPENSSL_NO_TLS1_2 + /* If no TLSv1.2 available then do nothing in this case */ + if (idx % 2 == 1) + return 1; +#endif + + if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(), + TLS_client_method(), TLS1_VERSION, + (idx % 2 == 0) ? TLS1_3_VERSION + : TLS1_2_VERSION, + &sctx, &cctx, cert, privkey)) + || !TEST_true(SSL_CTX_set_options(sctx, SSL_OP_NO_TICKET))) + goto end; + + SSL_CTX_sess_set_get_cb(sctx, get_session_cb); + get_sess_val = NULL; + + SSL_CTX_sess_set_cache_size(sctx, 1); + + if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, + NULL, NULL))) + goto end; + + if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))) + goto end; + + if (idx > 1) { + sess = SSL_get_session(serverssl); + if (!TEST_ptr(sess)) + goto end; + + /* + * Cause this session to have a longer timeout than the next session to + * be added. + */ + if (!TEST_true(SSL_SESSION_set_timeout(sess, LONG_MAX / 2))) { + sess = NULL; + goto end; + } + sess = NULL; + } + + SSL_shutdown(serverssl); + SSL_shutdown(clientssl); + SSL_free(serverssl); + SSL_free(clientssl); + serverssl = clientssl = NULL; + + /* + * Session cache size is 1 and we already populated the cache with a session + * so the next connection should cause an overflow. + */ + + if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, + NULL, NULL))) + goto end; + + if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))) + goto end; + + /* + * The session we just negotiated may have been already removed from the + * internal cache - but we will return it anyway from our external cache. + */ + get_sess_val = SSL_get_session(serverssl); + if (!TEST_ptr(get_sess_val)) + goto end; + sess = SSL_get1_session(clientssl); + if (!TEST_ptr(sess)) + goto end; + + SSL_shutdown(serverssl); + SSL_shutdown(clientssl); + SSL_free(serverssl); + SSL_free(clientssl); + serverssl = clientssl = NULL; + + if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, + NULL, NULL))) + goto end; + + if (!TEST_true(SSL_set_session(clientssl, sess))) + goto end; + + if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))) + goto end; + + testresult = 1; + + end: + SSL_free(serverssl); + SSL_free(clientssl); + SSL_CTX_free(sctx); + SSL_CTX_free(cctx); + SSL_SESSION_free(sess); + + return testresult; +} +#endif /* !defined(OPENSSL_NO_TLS1_3) || !defined(OPENSSL_NO_TLS1_2) */ + +struct resume_servername_cb_data { + int i; + SSL_CTX *cctx; + SSL_CTX *sctx; + SSL_SESSION *sess; + int recurse; +}; + +/* + * Servername callback. We use it here to run another complete handshake using + * the same session - and mark the session as not_resuamble at the end + */ +static int resume_servername_cb(SSL *s, int *ad, void *arg) +{ + struct resume_servername_cb_data *cbdata = arg; + SSL *serverssl = NULL, *clientssl = NULL; + int ret = SSL_TLSEXT_ERR_ALERT_FATAL; + + if (cbdata->recurse) + return SSL_TLSEXT_ERR_ALERT_FATAL; + + if ((cbdata->i % 3) != 1) + return SSL_TLSEXT_ERR_OK; + + cbdata->recurse = 1; + + if (!TEST_true(create_ssl_objects(cbdata->sctx, cbdata->cctx, &serverssl, + &clientssl, NULL, NULL)) + || !TEST_true(SSL_set_session(clientssl, cbdata->sess))) + goto end; + + ERR_set_mark(); + /* + * We expect this to fail - because the servername cb will fail. This will + * mark the session as not_resumable. + */ + if (!TEST_false(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))) { + ERR_clear_last_mark(); + goto end; + } + ERR_pop_to_mark(); + + ret = SSL_TLSEXT_ERR_OK; + end: + SSL_free(serverssl); + SSL_free(clientssl); + cbdata->recurse = 0; + return ret; +} +/* + * Test multiple resumptions and cache size handling + * Test 0: TLSv1.3 (max_early_data set) + * Test 1: TLSv1.3 (SSL_OP_NO_TICKET set) + * Test 2: TLSv1.3 (max_early_data and SSL_OP_NO_TICKET set) + * Test 3: TLSv1.3 (SSL_OP_NO_TICKET, simultaneous resumes) + * Test 4: TLSv1.2 + */ +static int test_multi_resume(int idx) +{ + SSL_CTX *sctx = NULL, *cctx = NULL; + SSL *serverssl = NULL, *clientssl = NULL; + SSL_SESSION *sess = NULL; + int max_version = TLS1_3_VERSION; + int i, testresult = 0; + struct resume_servername_cb_data cbdata; + +#if defined(OPENSSL_NO_TLS1_2) + if (idx == 4) + return 1; +#else + if (idx == 4) + max_version = TLS1_2_VERSION; +#endif +#if defined(OPENSSL_NO_TLS1_3) + if (idx != 4) + return 1; +#endif + + if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(), + TLS_client_method(), TLS1_VERSION, + max_version, &sctx, &cctx, cert, + privkey))) + goto end; + + /* + * TLSv1.3 only uses a session cache if either max_early_data > 0 (used for + * replay protection), or if SSL_OP_NO_TICKET is in use + */ + if (idx == 0 || idx == 2) { + if (!TEST_true(SSL_CTX_set_max_early_data(sctx, 1024))) + goto end; + } + if (idx == 1 || idx == 2 || idx == 3) + SSL_CTX_set_options(sctx, SSL_OP_NO_TICKET); + + SSL_CTX_sess_set_cache_size(sctx, 5); + + if (idx == 3) { + SSL_CTX_set_tlsext_servername_callback(sctx, resume_servername_cb); + SSL_CTX_set_tlsext_servername_arg(sctx, &cbdata); + cbdata.cctx = cctx; + cbdata.sctx = sctx; + cbdata.recurse = 0; + } + + for (i = 0; i < 30; i++) { + if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, + NULL, NULL)) + || !TEST_true(SSL_set_session(clientssl, sess))) + goto end; + + /* + * Check simultaneous resumes. We pause the connection part way through + * the handshake by (mis)using the servername_cb. The pause occurs after + * session resumption has already occurred, but before any session + * tickets have been issued. While paused we run another complete + * handshake resuming the same session. + */ + if (idx == 3) { + cbdata.i = i; + cbdata.sess = sess; + } + + /* + * Recreate a bug where dynamically changing the max_early_data value + * can cause sessions in the session cache which cannot be deleted. + */ + if ((idx == 0 || idx == 2) && (i % 3) == 2) + SSL_set_max_early_data(serverssl, 0); + + if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))) + goto end; + + if (sess == NULL || (idx == 0 && (i % 3) == 2)) { + if (!TEST_false(SSL_session_reused(clientssl))) + goto end; + } else { + if (!TEST_true(SSL_session_reused(clientssl))) + goto end; + } + SSL_SESSION_free(sess); + + /* Do a full handshake, followed by two resumptions */ + if ((i % 3) == 2) { + sess = NULL; + } else { + if (!TEST_ptr((sess = SSL_get1_session(clientssl)))) + goto end; + } + + SSL_shutdown(clientssl); + SSL_shutdown(serverssl); + SSL_free(serverssl); + SSL_free(clientssl); + serverssl = clientssl = NULL; + } + + /* We should never exceed the session cache size limit */ + if (!TEST_long_le(SSL_CTX_sess_number(sctx), 5)) + goto end; + + testresult = 1; + end: + SSL_free(serverssl); + SSL_free(clientssl); + SSL_CTX_free(sctx); + SSL_CTX_free(cctx); + SSL_SESSION_free(sess); + return testresult; +} + int setup_tests(void) { if (!TEST_ptr(certsdir = test_get_argument(0)) @@ -7264,6 +7580,10 @@ int setup_tests(void) #ifndef OPENSSL_NO_CERT_COMPRESSION ADD_ALL_TESTS(test_tls_cert_compression_api, 5); #endif +#if !defined(OPENSSL_NO_TLS1_3) || !defined(OPENSSL_NO_TLS1_2) + ADD_ALL_TESTS(test_session_cache_overflow, 4); +#endif + ADD_ALL_TESTS(test_multi_resume, 5); return 1; }