ssl: handle session ID cache lock failure

SSL_has_matching_session_id() returns 0 both when no matching session
exists and when the cache lock cannot be acquired. Its use in the
built-in generator's retry loop and the final uniqueness check can
therefore treat a lock failure as proof that no conflict exists.

Add an internal tri-state lookup. The final uniqueness check uses it as
a backstop for IDs produced by both built-in and application callbacks,
recording an internal fatal error when the cache cannot be checked. The
retry loop also uses it to prevent the built-in generator from returning
an ID after its own lookup failed, through the existing callback failure
path. Preserve the public API's boolean contract.

Correct the callback-selection cache lock failure reason from
SSL_R_SESSION_ID_CONTEXT_UNINITIALIZED to ERR_R_INTERNAL_ERROR.

Remove a redundant caller-side SSLfatal() that appended a generic
ERR_R_INTERNAL_ERROR after the specific error already recorded. Add a
deterministic unit test for final cache lookup failure.

Fixes #32358

Assisted-by: Codex:gpt-5.6-sol
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Dmitry Belyavskiy <[email protected]>
Merge-date: Wed Oct 21 16:25:29 2026
Merged-from: https://github.com/openssl/openssl/pull/32359

6 files changed, 177 insertions, 11 deletions

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 9b39de702c2..ff039c71388 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1158,8 +1158,11 @@ int SSL_set_generate_session_id(SSL *ssl, GEN_SESSION_CB cb)
     return 1;
 }
 
-int SSL_has_matching_session_id(const SSL *ssl, const unsigned char *id,
-    unsigned int id_len)
+/*
+ * Returns -1 on error, 0 if there is no match and 1 if there is a match.
+ */
+int ssl_has_matching_session_id(const SSL_CONNECTION *sc,
+    const unsigned char *id, unsigned int id_len)
 {
     /*
      * A quick examination of SSL_SESSION_hash and SSL_SESSION_cmp shows how
@@ -1169,22 +1172,30 @@ int SSL_has_matching_session_id(const SSL *ssl, const unsigned char *id,
      * by this SSL.
      */
     SSL_SESSION r, *p;
-    const SSL_CONNECTION *sc = SSL_CONNECTION_FROM_CONST_SSL(ssl);
 
     if (sc == NULL || id_len > sizeof(r.session_id))
-        return 0;
+        return -1;
 
     r.ssl_version = sc->version;
     r.session_id_length = id_len;
     memcpy(r.session_id, id, id_len);
 
     if (!CRYPTO_THREAD_read_lock(sc->session_ctx->lock))
-        return 0;
+        return -1;
     p = lh_SSL_SESSION_retrieve(sc->session_ctx->sessions, &r);
     CRYPTO_THREAD_unlock(sc->session_ctx->lock);
     return (p != NULL);
 }
 
+int SSL_has_matching_session_id(const SSL *ssl, const unsigned char *id,
+    unsigned int id_len)
+{
+    const SSL_CONNECTION *sc = SSL_CONNECTION_FROM_CONST_SSL(ssl);
+
+    /* Preserve the public API's boolean return value on internal errors. */
+    return ssl_has_matching_session_id(sc, id, id_len) > 0;
+}
+
 int SSL_CTX_set_purpose(SSL_CTX *s, int purpose)
 {
     return X509_VERIFY_PARAM_set_purpose(s->param, purpose);
diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
index 205f4d2275b..6ba7c549118 100644
--- a/ssl/ssl_local.h
+++ b/ssl/ssl_local.h
@@ -2899,6 +2899,8 @@ __owur CERT *ssl_cert_new(size_t ssl_pkey_num);
 __owur CERT *ssl_cert_dup(CERT *cert);
 void ssl_cert_clear_certs(CERT *c);
 void ssl_cert_free(CERT *c);
+__owur int ssl_has_matching_session_id(const SSL_CONNECTION *sc,
+    const unsigned char *id, unsigned int id_len);
 __owur int ssl_generate_session_id(SSL_CONNECTION *s, SSL_SESSION *ss);
 __owur int ssl_get_new_session(SSL_CONNECTION *s, int session);
 __owur SSL_SESSION *lookup_sess_in_cache(SSL_CONNECTION *s,
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 7a74b5bbefd..99d35f5f896 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -328,7 +328,10 @@ unsigned int SSL_SESSION_get_compress_id(const SSL_SESSION *s)
 static int def_generate_session_id(SSL *ssl, unsigned char *id,
     unsigned int *id_len)
 {
+    SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(ssl);
+    int match;
     unsigned int retry = 0;
+
     do {
         if (RAND_bytes_ex(ssl->ctx->libctx, id, *id_len, 0) <= 0)
             return 0;
@@ -337,7 +340,10 @@ static int def_generate_session_id(SSL *ssl, unsigned char *id,
             id[0]++;
         }
 #endif
-    } while (SSL_has_matching_session_id(ssl, id, *id_len) && (++retry < MAX_SESS_ID_ATTEMPTS));
+        match = ssl_has_matching_session_id(sc, id, *id_len);
+        if (match < 0)
+            return 0;
+    } while (match > 0 && (++retry < MAX_SESS_ID_ATTEMPTS));
     if (retry < MAX_SESS_ID_ATTEMPTS)
         return 1;
     /* else - woops a session_id match */
@@ -354,6 +360,7 @@ static int def_generate_session_id(SSL *ssl, unsigned char *id,
 
 int ssl_generate_session_id(SSL_CONNECTION *s, SSL_SESSION *ss)
 {
+    int match;
     unsigned int tmp;
     GEN_SESSION_CB cb = def_generate_session_id;
     SSL *ssl = SSL_CONNECTION_GET_SSL(s);
@@ -401,8 +408,7 @@ int ssl_generate_session_id(SSL_CONNECTION *s, SSL_SESSION *ss)
     }
     if (!CRYPTO_THREAD_read_lock(s->session_ctx->lock)) {
         CRYPTO_THREAD_unlock(ssl->lock);
-        SSLfatal(s, SSL_AD_INTERNAL_ERROR,
-            SSL_R_SESSION_ID_CONTEXT_UNINITIALIZED);
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         return 0;
     }
     if (s->generate_session_id)
@@ -432,8 +438,13 @@ int ssl_generate_session_id(SSL_CONNECTION *s, SSL_SESSION *ss)
     }
     ss->session_id_length = tmp;
     /* Finally, check for a conflict */
-    if (SSL_has_matching_session_id(ssl, ss->session_id,
-            (unsigned int)ss->session_id_length)) {
+    match = ssl_has_matching_session_id(s, ss->session_id,
+        (unsigned int)ss->session_id_length);
+    if (match < 0) {
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+    if (match > 0) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_SSL_SESSION_ID_CONFLICT);
         return 0;
     }
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 82a18721f40..bc00da04d1a 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -1431,7 +1431,7 @@ static int final_server_name(SSL_CONNECTION *s, unsigned int context, int sent)
                 ss->ext.tick_lifetime_hint = 0;
                 ss->ext.tick_age_add = 0;
                 if (!ssl_generate_session_id(s, ss)) {
-                    SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+                    /* SSLfatal() already called */
                     return 0;
                 }
             } else {
diff --git a/test/unit/build.info b/test/unit/build.info
index 76598e358ec..e086f0ed3d6 100644
--- a/test/unit/build.info
+++ b/test/unit/build.info
@@ -68,6 +68,12 @@ IF[{- $config{target} =~ /^(?:linux|BSD)/ -}]
   WRAP[crypto/bio/test_bss_sock]=read write BIO_closesocket
   INCLUDE[crypto/bio/test_bss_sock]=../../include ../../crypto/bio
   DEPEND[crypto/bio/test_bss_sock]=../../libcrypto.a
+
+  PROGRAMS{noinst}=ssl/test_ssl_sess
+  SOURCE[ssl/test_ssl_sess]=ssl/test_ssl_sess.c
+  INCLUDE[ssl/test_ssl_sess]=../../include ../../ssl
+  DEPEND[ssl/test_ssl_sess]=../../libssl.a ../../libcrypto.a
+  WRAP[ssl/test_ssl_sess]=CRYPTO_THREAD_read_lock
 ENDIF
 
 IF[{- $config{target} =~ /^VC-/ -}]
diff --git a/test/unit/ssl/test_ssl_sess.c b/test/unit/ssl/test_ssl_sess.c
new file mode 100644
index 00000000000..7a7c7d4cc3d
--- /dev/null
+++ b/test/unit/ssl/test_ssl_sess.c
@@ -0,0 +1,136 @@
+/*
+ * Copyright 2026 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License 2.0 (the "License").  You may not use
+ * this file except in compliance with the License.  You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <stdlib.h>
+#include <string.h>
+#include <cmocka.h>
+
+#include <openssl/crypto.h>
+#include <openssl/err.h>
+#include <openssl/ssl.h>
+
+#include "internal/ssl_unwrap.h"
+#include "ssl_local.h"
+
+typedef struct {
+    SSL_CTX *ctx;
+    SSL *ssl;
+    SSL_CONNECTION *sc;
+    SSL_SESSION *session;
+} SESSION_ID_FIXTURE;
+
+/*
+ * Fail one explicitly armed lock and pass all other lock calls through. The
+ * callback arms the next cache read, which is the final collision check.
+ */
+static CRYPTO_RWLOCK *read_lock_to_fail;
+
+/* wraps */
+
+int __real_CRYPTO_THREAD_read_lock(CRYPTO_RWLOCK *lock);
+int __wrap_CRYPTO_THREAD_read_lock(CRYPTO_RWLOCK *lock);
+
+int __wrap_CRYPTO_THREAD_read_lock(CRYPTO_RWLOCK *lock)
+{
+    if (lock != NULL && lock == read_lock_to_fail) {
+        read_lock_to_fail = NULL;
+        return 0;
+    }
+
+    return __real_CRYPTO_THREAD_read_lock(lock);
+}
+
+/* fault injection */
+
+static void fail_next_read_lock(CRYPTO_RWLOCK *lock)
+{
+    assert_null(read_lock_to_fail);
+    assert_non_null(lock);
+    read_lock_to_fail = lock;
+}
+
+/* helpers */
+
+static int generate_session_id_then_fail_cache_lock(SSL *ssl,
+    unsigned char *id, unsigned int *id_len)
+{
+    SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(ssl);
+
+    memset(id, 0x5a, *id_len);
+    fail_next_read_lock(sc->session_ctx->lock);
+    return 1;
+}
+
+/* fixtures */
+
+static int setup(void **state)
+{
+    SESSION_ID_FIXTURE *fixture = calloc(1, sizeof(*fixture));
+
+    assert_non_null(fixture);
+    fixture->ctx = SSL_CTX_new(TLS_server_method());
+    assert_non_null(fixture->ctx);
+    fixture->ssl = SSL_new(fixture->ctx);
+    assert_non_null(fixture->ssl);
+    fixture->sc = SSL_CONNECTION_FROM_SSL(fixture->ssl);
+    assert_non_null(fixture->sc);
+    fixture->session = SSL_SESSION_new();
+    assert_non_null(fixture->session);
+
+    fixture->sc->version = TLS1_2_VERSION;
+    read_lock_to_fail = NULL;
+    ERR_clear_error();
+    *state = fixture;
+    return 0;
+}
+
+static int teardown(void **state)
+{
+    SESSION_ID_FIXTURE *fixture = *state;
+
+    assert_null(read_lock_to_fail);
+    SSL_SESSION_free(fixture->session);
+    SSL_free(fixture->ssl);
+    SSL_CTX_free(fixture->ctx);
+    free(fixture);
+    ERR_clear_error();
+    return 0;
+}
+
+static void test_generate_session_id_final_cache_lock_failure(void **state)
+{
+    SESSION_ID_FIXTURE *fixture = *state;
+    unsigned long err;
+
+    assert_true(SSL_set_generate_session_id(fixture->ssl,
+        generate_session_id_then_fail_cache_lock));
+
+    assert_false(ssl_generate_session_id(fixture->sc, fixture->session));
+    /* Inspect the first error recorded for this failure. */
+    err = ERR_peek_error();
+    assert_true(ossl_statem_in_error(fixture->sc));
+    assert_int_equal(ERR_GET_LIB(err), ERR_LIB_SSL);
+    assert_int_equal(ERR_GET_REASON(err), ERR_R_INTERNAL_ERROR);
+}
+
+int main(void)
+{
+    const struct CMUnitTest tests[] = {
+        cmocka_unit_test_setup_teardown(
+            test_generate_session_id_final_cache_lock_failure, setup,
+            teardown),
+    };
+
+    cmocka_set_message_output(CM_OUTPUT_TAP);
+
+    return cmocka_run_group_tests(tests, NULL, NULL);
+}