diff --git a/src/core/tsi/ssl/session_cache/ssl_session_cache.cc b/src/core/tsi/ssl/session_cache/ssl_session_cache.cc index 5427c989b2f..d4bafaa0538 100644 --- a/src/core/tsi/ssl/session_cache/ssl_session_cache.cc +++ b/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) { diff --git a/test/core/tsi/ssl_session_cache_test.cc b/test/core/tsi/ssl_session_cache_test.cc index 9a7da12a9f3..5890f15bc63 100644 --- a/test/core/tsi/ssl_session_cache_test.cc +++ b/test/core/tsi/ssl_session_cache_test.cc @@ -103,6 +103,7 @@ TEST(SslSessionCacheTest, LruCache) { { RefCountedPtr 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 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 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 diff --git a/test/cpp/end2end/ssl_credentials_test.cc b/test/cpp/end2end/ssl_credentials_test.cc index 91022228986..b1a51dafccd 100644 --- a/test/cpp/end2end/ssl_credentials_test.cc +++ b/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(¬ification); }); + 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(¬ification); }); + 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