[ssl] Do not crash if ssl session cache capacity is zero. (#34539)

This behavior is dangerous because we will crash when the cache is
created, which is not necessarily on application startup and is likely
when you first try to establish an SSL connection. Instead, we log an
error. If the SSL library attempts to put a session ticket in the cache
it will fail to do so, but everything else will continue as normal. In
particular, we will always seamlessly fall back to a full SSL handshake.

Along the way, we also ensure that you cannot put a null `SSL_SESSION`
into the cache, which would lead to a segfault when it is fetched from
the cache.

src/core/tsi/ssl/session_cache/ssl_session_cache.cc

@@ -62,7 +62,11 @@ class SslSessionLRUCache::Node {
 };
 
 SslSessionLRUCache::SslSessionLRUCache(size_t capacity) : capacity_(capacity) {
-  GPR_ASSERT(capacity > 0);
+  if (capacity == 0) {
+    gpr_log(
+        GPR_ERROR,
+        "SslSessionLRUCache capacity is zero. SSL sessions cannot be resumed.");
+  }
 }
 
 SslSessionLRUCache::~SslSessionLRUCache() {
@@ -94,6 +98,10 @@ SslSessionLRUCache::Node* SslSessionLRUCache::FindLocked(
 }
 
 void SslSessionLRUCache::Put(const char* key, SslSessionPtr session) {
+  if (session == nullptr) {
+    gpr_log(GPR_ERROR, "Attempted to put null SSL session in session cache.");
+    return;
+  }
   grpc_core::MutexLock lock(&lock_);
   Node* node = FindLocked(key);
   if (node != nullptr) {

test/core/tsi/ssl_session_cache_test.cc

@@ -103,6 +103,7 @@ TEST(SslSessionCacheTest, LruCache) {
   {
     RefCountedPtr<tsi::SslSessionLRUCache> cache =
         tsi::SslSessionLRUCache::Create(3);
+    EXPECT_EQ(cache->Size(), 0);
     tsi::SslSessionPtr sess2 = tracker.NewSession(2);
     SSL_SESSION* sess2_ptr = sess2.get();
     cache->Put("first.dropbox.com", std::move(sess2));
@@ -143,6 +144,45 @@ TEST(SslSessionCacheTest, LruCache) {
   EXPECT_EQ(tracker.AliveCount(), 0);
 }
 
+TEST(SslSessionCacheTest, PutAndGet) {
+  // Set up an empty cache and an SSL session.
+  SSL_CTX* ssl_ctx = SSL_CTX_new(TLS_method());
+  tsi::SslSessionPtr ssl_session_ptr(SSL_SESSION_new(ssl_ctx));
+  RefCountedPtr<tsi::SslSessionLRUCache> cache =
+      tsi::SslSessionLRUCache::Create(1);
+  EXPECT_EQ(cache->Size(), 0);
+  // Put the SSL session in the cache.
+  cache->Put("foo.domain", std::move(ssl_session_ptr));
+  EXPECT_EQ(cache->Size(), 1);
+  // Get a copy of the SSL session from the cache.
+  EXPECT_EQ(cache->Size(), 1);
+  EXPECT_NE(cache->Get("foo.domain"), nullptr);
+  // Try to put a null SSL session in the cache and check that it was not
+  // successful.
+  cache->Put("foo.domain.2", /*session=*/nullptr);
+  EXPECT_EQ(cache->Size(), 1);
+  EXPECT_NE(cache->Get("foo.domain"), nullptr);
+  EXPECT_EQ(cache->Get("foo.domain.2"), nullptr);
+  // Cleanup.
+  SSL_CTX_free(ssl_ctx);
+}
+
+TEST(SslSessionCacheTest, CapacityZeroCache) {
+  // Set up an empty cache and an SSL session.
+  SSL_CTX* ssl_ctx = SSL_CTX_new(TLS_method());
+  tsi::SslSessionPtr ssl_session_ptr(SSL_SESSION_new(ssl_ctx));
+  RefCountedPtr<tsi::SslSessionLRUCache> cache =
+      tsi::SslSessionLRUCache::Create(0);
+  EXPECT_EQ(cache->Size(), 0);
+  // Try to put the SSL session in the cache and check that it was not
+  // successful.
+  cache->Put("foo.domain", std::move(ssl_session_ptr));
+  EXPECT_EQ(cache->Size(), 0);
+  EXPECT_EQ(cache->Get("foo.domain"), nullptr);
+  // Cleanup.
+  SSL_CTX_free(ssl_ctx);
+}
+
 }  // namespace
 }  // namespace grpc_core
 

test/cpp/end2end/ssl_credentials_test.cc

@@ -125,6 +125,31 @@ void DoRpc(const std::string& server_addr,
   }
 }
 
+TEST_F(SslCredentialsTest, SequentialResumption) {
+  server_addr_ = absl::StrCat("localhost:",
+                              std::to_string(grpc_pick_unused_port_or_die()));
+  absl::Notification notification;
+  server_thread_ = new std::thread([&]() { RunServer(&notification); });
+  notification.WaitForNotification();
+
+  std::string root_cert = ReadFile(kCaCertPath);
+  std::string client_key = ReadFile(kClientKeyPath);
+  std::string client_cert = ReadFile(kClientCertPath);
+  grpc::SslCredentialsOptions ssl_options;
+  ssl_options.pem_root_certs = root_cert;
+  ssl_options.pem_private_key = client_key;
+  ssl_options.pem_cert_chain = client_cert;
+
+  grpc_ssl_session_cache* cache = grpc_ssl_session_cache_create_lru(16);
+
+  DoRpc(server_addr_, ssl_options, cache, /*expect_session_reuse=*/false);
+  for (int i = 0; i < 10; i++) {
+    DoRpc(server_addr_, ssl_options, cache, /*expect_session_reuse=*/true);
+  }
+
+  grpc_ssl_session_cache_destroy(cache);
+}
+
 TEST_F(SslCredentialsTest, ConcurrentResumption) {
   server_addr_ = absl::StrCat("localhost:",
                               std::to_string(grpc_pick_unused_port_or_die()));
@@ -157,6 +182,29 @@ TEST_F(SslCredentialsTest, ConcurrentResumption) {
   grpc_ssl_session_cache_destroy(cache);
 }
 
+TEST_F(SslCredentialsTest, ResumptionFailsDueToNoCapacityInCache) {
+  server_addr_ = absl::StrCat("localhost:",
+                              std::to_string(grpc_pick_unused_port_or_die()));
+  absl::Notification notification;
+  server_thread_ = new std::thread([&]() { RunServer(&notification); });
+  notification.WaitForNotification();
+
+  std::string root_cert = ReadFile(kCaCertPath);
+  std::string client_key = ReadFile(kClientKeyPath);
+  std::string client_cert = ReadFile(kClientCertPath);
+  grpc::SslCredentialsOptions ssl_options;
+  ssl_options.pem_root_certs = root_cert;
+  ssl_options.pem_private_key = client_key;
+  ssl_options.pem_cert_chain = client_cert;
+
+  grpc_ssl_session_cache* cache = grpc_ssl_session_cache_create_lru(0);
+
+  DoRpc(server_addr_, ssl_options, cache, /*expect_session_reuse=*/false);
+  DoRpc(server_addr_, ssl_options, cache, /*expect_session_reuse=*/false);
+
+  grpc_ssl_session_cache_destroy(cache);
+}
+
 }  // namespace
 }  // namespace testing
 }  // namespace grpc