diff --git a/CHANGES b/CHANGES
index 60551db8e..647362367 100644
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,8 @@
Changes between 8.4.0 and 8.5.0 [xx XXX xxxx]
+ *) 修复CVE-2024-2511
+
*) 修复CVE-2023-6129
*) 修复CVE-2023-6237
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index a442bb8ef..03f2f051f 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -3840,9 +3840,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_sess.c b/ssl/ssl_sess.c
index 6bbd27d4e..de1386c02 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -157,16 +157,11 @@ SSL_SESSION *SSL_SESSION_new(void)
return ss;
}
-SSL_SESSION *SSL_SESSION_dup(const 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(const SSL_SESSION *src, int ticket)
+static SSL_SESSION *ssl_session_dup_intern(const SSL_SESSION *src, int ticket)
{
SSL_SESSION *dest;
@@ -310,6 +305,27 @@ SSL_SESSION *ssl_session_dup(const SSL_SESSION *src, int ticket)
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)
@@ -563,6 +579,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;
+ }
ssl_tsan_counter(s->session_ctx,
&s->session_ctx->stats.sess_cb_hit);
@@ -824,25 +846,17 @@ int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *c)
c->time = time(NULL);
ssl_session_calculate_timeout(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;
- } 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
@@ -850,6 +864,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 4422bffa9..11d3566bd 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -2522,9 +2522,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 4cd8855cf..f891e646d 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -2133,6 +2133,32 @@ static int execute_test_session(int maxprot, int use_int_cache,
goto end;
}
}
+ /*
+ * 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))
+ || !TEST_ptr(sess1->owner)
+ || !TEST_ptr_null(sess2->owner))
+ goto end;
testresult = 1;
@@ -2210,7 +2236,6 @@ static int test_session_wo_ca_names(void)
#endif
}
-
#ifndef OSSL_NO_USABLE_TLS1_3
static SSL_SESSION *sesscache[6];
static int do_cache;
@@ -8596,6 +8621,126 @@ static int test_session_timeout(int test)
return testresult;
}
+/*
+ * 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(OSSL_NO_USABLE_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 OSSL_NO_USABLE_TLS1_3
+ /* If no TLSv1.3 available then do nothing in this case */
+ if (idx % 2 == 0)
+ return TEST_skip("No TLSv1.3 available");
+#endif
+#ifdef OPENSSL_NO_TLS1_2
+ /* If no TLSv1.2 available then do nothing in this case */
+ if (idx % 2 == 1)
+ return TEST_skip("No TLSv1.2 available");
+#endif
+
+ if (!TEST_true(create_ssl_ctx_pair(libctx, 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(OSSL_NO_USABLE_TLS1_3) || !defined(OPENSSL_NO_TLS1_2) */
+
/*
* Test 0: Client sets servername and server acknowledges it (TLSv1.2)
* Test 1: Client sets servername and server does not acknowledge it (TLSv1.2)
@@ -10017,6 +10162,177 @@ static int test_tls_cert_compression_api(int tst)
}
#endif
+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 TEST_skip("TLSv1.2 is disabled in this build");
+#else
+ if (idx == 4)
+ max_version = TLS1_2_VERSION;
+#endif
+#if defined(OSSL_NO_USABLE_TLS1_3)
+ if (idx != 4)
+ return TEST_skip("No usable TLSv1.3 in this build");
+#endif
+
+ if (!TEST_true(create_ssl_ctx_pair(libctx, 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;
+}
+
OPT_TEST_DECLARE_USAGE("certfile privkeyfile srpvfile tmpfile provider config dhfile\n")
int setup_tests(void)
@@ -10284,7 +10600,11 @@ int setup_tests(void)
ADD_TEST(test_set_verify_cert_store_ssl_ctx);
ADD_TEST(test_set_verify_cert_store_ssl);
ADD_ALL_TESTS(test_session_timeout, 1);
+#if !defined(OSSL_NO_USABLE_TLS1_3) || !defined(OPENSSL_NO_TLS1_2)
+ ADD_ALL_TESTS(test_session_cache_overflow, 4);
+#endif
ADD_TEST(test_load_dhfile);
+ ADD_ALL_TESTS(test_multi_resume, 5);
return 1;
err: