Fix data race between SSL_SESSION_list_add and ssl_session_dup

Reviewed-by: Neil Horman <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from openssl#24673)
ClickHouse · Jun 21, 2024 · 5d81fa7 · 5d81fa7
1 parent e3eca22
commit 5d81fa7
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 7 deletions.
diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
@@ -532,7 +532,6 @@ struct ssl_session_st {
      * certificate is not ok, we must remember the error for session reuse:
      */
     long verify_result;         /* only for servers */
-    CRYPTO_REF_COUNT references;
     OSSL_TIME timeout;
     OSSL_TIME time;
     OSSL_TIME calc_timeout;
@@ -542,11 +541,6 @@ struct ssl_session_st {
                                  * load the 'cipher' structure */
     unsigned int kex_group;      /* TLS group from key exchange */
     CRYPTO_EX_DATA ex_data;     /* application specific data */
-    /*
-     * These are used to make removal of session-ids more efficient and to
-     * implement a maximum cache size.
-     */
-    struct ssl_session_st *prev, *next;
 
     struct {
         char *hostname;
@@ -576,6 +570,13 @@ struct ssl_session_st {
     size_t ticket_appdata_len;
     uint32_t flags;
     SSL_CTX *owner;
+
+    /*
+     * These are used to make removal of session-ids more efficient and to
+     * implement a maximum cache size. Access requires protection of ctx->lock.
+     */
+    struct ssl_session_st *prev, *next;
+    CRYPTO_REF_COUNT references;
 };
 
 /* Extended master secret support */

diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
@@ -143,7 +143,12 @@ SSL_SESSION *ssl_session_dup(const SSL_SESSION *src, int ticket)
     dest = OPENSSL_malloc(sizeof(*dest));
     if (dest == NULL)
         return NULL;
-    memcpy(dest, src, sizeof(*dest));
+
+    /*
+     * src is logically read-only but the prev/next pointers are not, they are
+     * part of the session cache and can be modified concurrently.
+     */
+    memcpy(dest, src, offsetof(SSL_SESSION, prev));
 
     /*
      * Set the various pointers to NULL so that we can call SSL_SESSION_free in