Allow a SSL_SESSION to only ever live in one SSL_CTX.

As SSL_SESSION is part of an intrusive linked list, it cannot be in more
than one chain at once.

Bug: 527997772
Change-Id: I381184d7641407edc4d593594d41efe26a6a6964
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/97869
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Rudolf Polzer <rpolzer@google.com>

include/openssl/ssl.h

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index eb4e772..5c6d17a 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2310,7 +2310,10 @@
 
 // SSL_CTX_add_session inserts `session` into `ctx`'s internal session cache. It
 // returns one on success and zero on error or if `session` is already in the
-// cache. The caller retains its reference to `session`.
+// cache. The caller retains its reference to `session`. A `session` can be
+// only in one `ctx` at any given time; it is an error to add it to more.
+//
+// TODO(crbug.com/527997772): Remove this restriction.
 OPENSSL_EXPORT int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *session);
 
 // SSL_CTX_remove_session removes `session` from `ctx`'s internal session cache.

ssl/ssl_session.cc

diff --git a/ssl/ssl_session.cc b/ssl/ssl_session.cc
index 0db1798..6c94441 100644
--- a/ssl/ssl_session.cc
+++ b/ssl/ssl_session.cc
@@ -718,6 +718,14 @@
 static bool add_session_locked(SSLContext *ctx,
                                UniquePtr<SSL_SESSION> session) {
   SSL_SESSION *new_session = session.get();
+
+  // Sessions have intrusive linked lists, so they cannot be stored in two
+  // SSLContexts at once.
+  // TODO(crbug.com/527997772): Remove this restriction.
+  if ((new_session->prev != nullptr || new_session->next != nullptr)) {
+    return false;
+  }
+
   SSL_SESSION *old_session;
   if (!lh_SSL_SESSION_insert(ctx->sessions, &old_session, new_session)) {
     return false;
@@ -730,11 +738,9 @@
   session.reset(old_session);
 
   if (old_session != nullptr) {
-    if (old_session == new_session) {
-      // `session` was already in the cache. There are no linked list pointers
-      // to update.
-      return false;
-    }
+    // The session cannot have been in the session, due to the check above.
+    // TODO(crbug.com/527997772): Revisit this when we remove the check.
+    BSSL_CHECK(old_session != new_session);
 
     // There was a session ID collision. `old_session` was replaced with
     // `session` in the hash table, so `old_session` must be removed from the

ssl/ssl_test.cc

diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 7975325..9c8101c 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -11961,5 +11961,23 @@
   ASSERT_TRUE(CompleteHandshakes(client.get(), server.get()));
 }
 
+TEST(SSLTest, SessionCrossContextCache) {
+  UniquePtr<SSL_CTX> ctx_a(SSL_CTX_new(TLS_method()));
+  UniquePtr<SSL_CTX> ctx_b(SSL_CTX_new(TLS_method()));
+  ASSERT_TRUE(ctx_a);
+  ASSERT_TRUE(ctx_b);
+
+  UniquePtr<SSL_SESSION> session(SSL_SESSION_new(ctx_b.get()));
+  ASSERT_TRUE(session);
+  uint8_t sid[32] = {};
+  ASSERT_TRUE(SSL_SESSION_set1_id(session.get(), sid, sizeof(sid)));
+
+  EXPECT_TRUE(SSL_CTX_add_session(ctx_a.get(), session.get()));
+
+  // This should fail because the session is already in ctx_a.
+  // TODO(crbug.com/527997772): Remove this restriction.
+  EXPECT_FALSE(SSL_CTX_add_session(ctx_b.get(), session.get()));
+}
+
 }  // namespace
 BSSL_NAMESPACE_END