commit 9eaa3bd55d5d7836f56051bf87c051ae6890ca06
author David Benjamin <davidben@google.com> Wed Sep 27 17:03:54 2017 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Wed Sep 27 21:58:05 2017 +0000
tree e84583a9dc29e836be28ee7f189b5e16f381b986
parent 73d42e614c21b3ec173c839bf1f687f1654b4265

Remove SSL_CTX_sessions and properly lock SSL_CTX_sess_number.

SSL_CTX_sessions is the only think making us expose LHASH as public API
and nothing uses it. Nothing can use it anyway as it's not thread-safe.
I haven't actually removed it yet since SSL_CTX is public, but once the
types are opaque, we could trim the number of symbols ssl.h pulls in
with some work.

Relatedly, fix thread safety of SSL_CTX_sess_number.

Change-Id: I75a6c93509d462cd5ed3ce76c587f0d1e7cd0797
Reviewed-on: https://boringssl-review.googlesource.com/20804
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

include/openssl/ssl.h

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index d6f6149..008a90f 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1854,9 +1854,6 @@
 // session cache.
 OPENSSL_EXPORT unsigned long SSL_CTX_sess_get_cache_size(const SSL_CTX *ctx);
 
-// SSL_CTX_sessions returns |ctx|'s internal session cache.
-OPENSSL_EXPORT LHASH_OF(SSL_SESSION) *SSL_CTX_sessions(SSL_CTX *ctx);
-
 // SSL_CTX_sess_number returns the number of sessions in |ctx|'s internal
 // session cache.
 OPENSSL_EXPORT size_t SSL_CTX_sess_number(const SSL_CTX *ctx);

ssl/ssl_lib.cc

diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 9c551c2..607bf07 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -1566,9 +1566,8 @@
          ssl->s3->send_connection_binding;
 }
 
-LHASH_OF(SSL_SESSION) *SSL_CTX_sessions(SSL_CTX *ctx) { return ctx->sessions; }
-
 size_t SSL_CTX_sess_number(const SSL_CTX *ctx) {
+  MutexReadLock lock(const_cast<CRYPTO_MUTEX *>(&ctx->lock));
   return lh_SSL_SESSION_num_items(ctx->sessions);
 }
 

ssl/ssl_session.cc

diff --git a/ssl/ssl_session.cc b/ssl/ssl_session.cc
index 64998d8..6c9db80 100644
--- a/ssl/ssl_session.cc
+++ b/ssl/ssl_session.cc
@@ -1041,7 +1041,8 @@
 
   // Enforce any cache size limits.
   if (SSL_CTX_sess_get_cache_size(ctx) > 0) {
-    while (SSL_CTX_sess_number(ctx) > SSL_CTX_sess_get_cache_size(ctx)) {
+    while (lh_SSL_SESSION_num_items(ctx->sessions) >
+           SSL_CTX_sess_get_cache_size(ctx)) {
       if (!remove_session_lock(ctx, ctx->session_cache_tail, 0)) {
         break;
       }

ssl/ssl_test.cc

diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 88d8e3f..2ee0124 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -1399,7 +1399,7 @@
 
   // Check the hash table.
   std::vector<SSL_SESSION*> actual, expected_copy;
-  lh_SSL_SESSION_doall_arg(SSL_CTX_sessions(ctx), AppendSession, &actual);
+  lh_SSL_SESSION_doall_arg(ctx->sessions, AppendSession, &actual);
   expected_copy = expected;
 
   std::sort(actual.begin(), actual.end());