[Security - TLS] DirectoryReloaderCrlProvider construction time bug fix (#35247)

The `DirectoryReloaderProvider` currently segfaults on construction if grpc_init() is not called before construction. This is because when creating the `DirectoryReloaderCrlProvider` we [call GetDefaultEventEngine](a58f3f2df5/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc (L152)), and getting the default event engine requires that `grpc_init` is called.

This PR adds a test that catches the segfault and adds `grpc_init` and `grpc_shutdown` to the ctor and dtor of `DirectoryReloaderCrlProvider` so that the test passes.

Closes #35247

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35247 from gtcooke94:crl_provider_init_fix 25f3dc7f27
PiperOrigin-RevId: 589885254

src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc

@@ -148,8 +148,7 @@ absl::StatusOr<std::shared_ptr<CrlProvider>> CreateDirectoryReloaderCrlProvider(
     return absl::InvalidArgumentError("Refresh duration minimum is 60 seconds");
   }
   auto provider = std::make_shared<DirectoryReloaderCrlProvider>(
-      refresh_duration, reload_error_callback,
-      grpc_event_engine::experimental::GetDefaultEventEngine(),
+      refresh_duration, reload_error_callback, /*event_engine=*/nullptr,
       MakeDirectoryReader(directory));
   // This could be slow to do at startup, but we want to
   // make sure it's done before the provider is used.
@@ -157,10 +156,28 @@ absl::StatusOr<std::shared_ptr<CrlProvider>> CreateDirectoryReloaderCrlProvider(
   return provider;
 }
 
+DirectoryReloaderCrlProvider::DirectoryReloaderCrlProvider(
+    std::chrono::seconds duration, std::function<void(absl::Status)> callback,
+    std::shared_ptr<grpc_event_engine::experimental::EventEngine> event_engine,
+    std::shared_ptr<DirectoryReader> directory_impl)
+    : refresh_duration_(Duration::FromSecondsAsDouble(duration.count())),
+      reload_error_callback_(std::move(callback)),
+      crl_directory_(std::move(directory_impl)) {
+  // Must be called before `GetDefaultEventEngine`
+  grpc_init();
+  if (event_engine == nullptr) {
+    event_engine_ = grpc_event_engine::experimental::GetDefaultEventEngine();
+  } else {
+    event_engine_ = std::move(event_engine);
+  }
+}
+
 DirectoryReloaderCrlProvider::~DirectoryReloaderCrlProvider() {
   if (refresh_handle_.has_value()) {
     event_engine_->Cancel(refresh_handle_.value());
   }
+  // Call here because we call grpc_init in the constructor
+  grpc_shutdown();
 }
 
 void DirectoryReloaderCrlProvider::UpdateAndStartTimer() {
@@ -209,9 +226,9 @@ absl::Status DirectoryReloaderCrlProvider::Update() {
     // in-place updated in crls_.
     for (auto& kv : new_crls) {
       std::shared_ptr<Crl>& crl = kv.second;
-      // It's not safe to say crl->Issuer() on the LHS and std::move(crl) on the
-      // RHS, because C++ does not guarantee which of those will be executed
-      // first.
+      // It's not safe to say crl->Issuer() on the LHS and std::move(crl) on
+      // the RHS, because C++ does not guarantee which of those will be
+      // executed first.
       std::string issuer(crl->Issuer());
       crls_[std::move(issuer)] = std::move(crl);
     }

src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h

@@ -98,11 +98,7 @@ class DirectoryReloaderCrlProvider
       std::chrono::seconds duration, std::function<void(absl::Status)> callback,
       std::shared_ptr<grpc_event_engine::experimental::EventEngine>
           event_engine,
-      std::shared_ptr<DirectoryReader> directory_impl)
-      : refresh_duration_(Duration::FromSecondsAsDouble(duration.count())),
-        reload_error_callback_(std::move(callback)),
-        event_engine_(std::move(event_engine)),
-        crl_directory_(std::move(directory_impl)) {}
+      std::shared_ptr<DirectoryReader> directory_impl);
 
   ~DirectoryReloaderCrlProvider() override;
   std::shared_ptr<Crl> GetCrl(const CertificateInfo& certificate_info) override;

test/cpp/end2end/crl_provider_test.cc

@@ -59,8 +59,23 @@ const char* kRevokedCertPath = "test/core/tsi/test_creds/crl_data/revoked.pem";
 const char* kValidKeyPath = "test/core/tsi/test_creds/crl_data/valid.key";
 const char* kValidCertPath = "test/core/tsi/test_creds/crl_data/valid.pem";
 const char* kRootCrlPath = "test/core/tsi/test_creds/crl_data/crls/current.crl";
+const char* kCrlDirectoryPath = "test/core/tsi/test_creds/crl_data/crls/";
 constexpr char kMessage[] = "Hello";
 
+// This test must be at the top of the file because the
+// DirectoryReloaderCrlProvider gets the default event engine on construction.
+// To get the default event engine, grpc_init must have been called, otherwise a
+// segfault occurs. This test checks that no segfault occurs while getting the
+// default event engine during the construction of a
+// DirectoryReloaderCrlProvider. `grpc_init` is global state, so if another test
+// runs first, then this test could pass because of another test modifying the
+// global state
+TEST(DirectoryReloaderCrlProviderTestNoFixture, Construction) {
+  auto provider = grpc_core::experimental::CreateDirectoryReloaderCrlProvider(
+      kCrlDirectoryPath, std::chrono::seconds(60), nullptr);
+  ASSERT_TRUE(provider.ok()) << provider.status();
+}
+
 class CrlProviderTest : public ::testing::Test {
  protected:
   void RunServer(absl::Notification* notification, absl::string_view server_key,
@@ -137,7 +152,7 @@ void DoRpc(const std::string& server_addr,
   }
 }
 
-TEST_F(CrlProviderTest, CrlProviderValid) {
+TEST_F(CrlProviderTest, CrlProviderValidStaticProvider) {
   server_addr_ = absl::StrCat("localhost:",
                               std::to_string(grpc_pick_unused_port_or_die()));
   absl::Notification notification;
@@ -220,6 +235,47 @@ TEST_F(CrlProviderTest, CrlProviderRevokedServer) {
   DoRpc(server_addr_, options, false);
 }
 
+TEST_F(CrlProviderTest, CrlProviderValidReloaderProvider) {
+  server_addr_ = absl::StrCat("localhost:",
+                              std::to_string(grpc_pick_unused_port_or_die()));
+  absl::Notification notification;
+  std::string server_key = grpc_core::testing::GetFileContents(kValidKeyPath);
+  std::string server_cert = grpc_core::testing::GetFileContents(kValidCertPath);
+  server_thread_ = new std::thread(
+      [&]() { RunServer(&notification, server_key, server_cert); });
+  notification.WaitForNotification();
+
+  std::string root_cert = grpc_core::testing::GetFileContents(kRootPath);
+  std::string client_key = grpc_core::testing::GetFileContents(kValidKeyPath);
+  std::string client_cert = grpc_core::testing::GetFileContents(kValidCertPath);
+  experimental::IdentityKeyCertPair key_cert_pair;
+  key_cert_pair.private_key = client_key;
+  key_cert_pair.certificate_chain = client_cert;
+  std::vector<experimental::IdentityKeyCertPair> identity_key_cert_pairs;
+  identity_key_cert_pairs.emplace_back(key_cert_pair);
+  auto certificate_provider =
+      std::make_shared<experimental::StaticDataCertificateProvider>(
+          root_cert, identity_key_cert_pairs);
+  grpc::experimental::TlsChannelCredentialsOptions options;
+  options.set_certificate_provider(certificate_provider);
+  options.watch_root_certs();
+  options.set_root_cert_name("root");
+  options.watch_identity_key_cert_pairs();
+  options.set_identity_cert_name("identity");
+
+  absl::StatusOr<std::shared_ptr<grpc_core::experimental::CrlProvider>>
+      provider = grpc_core::experimental::CreateDirectoryReloaderCrlProvider(
+          kCrlDirectoryPath, std::chrono::seconds(60), nullptr);
+  ASSERT_TRUE(provider.ok());
+
+  options.set_crl_provider(*provider);
+  options.set_check_call_host(false);
+  auto verifier = std::make_shared<experimental::NoOpCertificateVerifier>();
+  options.set_certificate_verifier(verifier);
+
+  DoRpc(server_addr_, options, true);
+}
+
 }  // namespace
 }  // namespace testing
 }  // namespace grpc