Revert "[Security - CrlProvider] Use a better mechanism for Crl Lookup and add some verification helpers (#35641)"

This reverts commit 310770d61d.

BUILD

@@ -3863,7 +3863,6 @@ grpc_cc_library(
     external_deps = [
         "absl/base:core_headers",
         "absl/status",
-        "absl/status:statusor",
         "absl/strings",
         "libcrypto",
         "libssl",

CMakeLists.txt

@@ -15927,7 +15927,6 @@ add_executable(grpc_tls_crl_provider_test
   test/core/event_engine/event_engine_test_utils.cc
   test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.cc
   test/core/security/grpc_tls_crl_provider_test.cc
-  test/core/tsi/transport_security_test_lib.cc
 )
 if(WIN32 AND MSVC)
   if(BUILD_SHARED_LIBS)
@@ -28085,7 +28084,6 @@ if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX)
 
   add_executable(ssl_transport_security_utils_test
     test/core/tsi/ssl_transport_security_utils_test.cc
-    test/core/tsi/transport_security_test_lib.cc
   )
   if(WIN32 AND MSVC)
     if(BUILD_SHARED_LIBS)

build_autogenerated.yaml

@@ -10769,14 +10769,12 @@ targets:
   headers:
   - test/core/event_engine/event_engine_test_utils.h
   - test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h
-  - test/core/tsi/transport_security_test_lib.h
   src:
   - test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.proto
   - test/core/util/fuzz_config_vars.proto
   - test/core/event_engine/event_engine_test_utils.cc
   - test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.cc
   - test/core/security/grpc_tls_crl_provider_test.cc
-  - test/core/tsi/transport_security_test_lib.cc
   deps:
   - gtest
   - protobuf
@@ -18036,11 +18034,9 @@ targets:
   gtest: true
   build: test
   language: c++
-  headers:
-  - test/core/tsi/transport_security_test_lib.h
+  headers: []
   src:
   - test/core/tsi/ssl_transport_security_utils_test.cc
-  - test/core/tsi/transport_security_test_lib.cc
   deps:
   - gtest
   - grpc_test_util

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

@@ -53,22 +53,15 @@ namespace grpc_core {
 namespace experimental {
 
 namespace {
-// TODO(gtcooke94) Move ssl_transport_security_utils to it's own BUILD target
-// and add this to it.
-absl::StatusOr<std::string> IssuerFromCrl(X509_CRL* crl) {
+std::string IssuerFromCrl(X509_CRL* crl) {
   if (crl == nullptr) {
-    return absl::InvalidArgumentError("crl cannot be null");
+    return "";
   }
-  X509_NAME* issuer = X509_CRL_get_issuer(crl);
-  if (issuer == nullptr) {
-    return absl::InvalidArgumentError("crl cannot have null issuer");
+  char* buf = X509_NAME_oneline(X509_CRL_get_issuer(crl), nullptr, 0);
+  std::string ret;
+  if (buf != nullptr) {
+    ret = buf;
   }
-  unsigned char* buf = nullptr;
-  int len = i2d_X509_NAME(issuer, &buf);
-  if (len < 0 || buf == nullptr) {
-    return absl::InvalidArgumentError("crl cannot have null issuer");
-  }
-  std::string ret(reinterpret_cast<char const*>(buf), len);
   OPENSSL_free(buf);
   return ret;
 }
@@ -110,11 +103,11 @@ absl::StatusOr<std::unique_ptr<Crl>> Crl::Parse(absl::string_view crl_string) {
 }
 
 absl::StatusOr<std::unique_ptr<CrlImpl>> CrlImpl::Create(X509_CRL* crl) {
-  absl::StatusOr<std::string> issuer = IssuerFromCrl(crl);
-  if (!issuer.ok()) {
-    return issuer.status();
+  std::string issuer = IssuerFromCrl(crl);
+  if (issuer.empty()) {
+    return absl::InvalidArgumentError("Issuer of crl cannot be empty");
   }
-  return std::make_unique<CrlImpl>(crl, *issuer);
+  return std::make_unique<CrlImpl>(crl, issuer);
 }
 
 CrlImpl::~CrlImpl() { X509_CRL_free(crl_); }

src/core/tsi/ssl_transport_security.cc

@@ -1004,14 +1004,15 @@ static int GetCrlFromProvider(X509_STORE_CTX* ctx, X509_CRL** crl_out,
   auto* provider = static_cast<grpc_core::experimental::CrlProvider*>(
       SSL_CTX_get_ex_data(ssl_ctx, g_ssl_ctx_ex_crl_provider_index));
 
-  absl::StatusOr<std::string> issuer_name = grpc_core::IssuerFromCert(cert);
-  if (!issuer_name.ok()) {
-    gpr_log(GPR_ERROR, "Could not get certificate issuer name");
+  char* buf = X509_NAME_oneline(X509_get_issuer_name(cert), nullptr, 0);
+  if (buf == nullptr) {
+    gpr_log(GPR_ERROR, "Certificate has null issuer, cannot do CRL lookup");
     return 0;
   }
-  grpc_core::experimental::CertificateInfoImpl cert_impl(*issuer_name);
+  grpc_core::experimental::CertificateInfoImpl cert_impl(buf);
   std::shared_ptr<grpc_core::experimental::Crl> internal_crl =
       provider->GetCrl(cert_impl);
+  OPENSSL_free(buf);
   // There wasn't a CRL found in the provider. Returning 0 will end up causing
   // OpenSSL to return X509_V_ERR_UNABLE_TO_GET_CRL. We then catch that error
   // and behave how we want for a missing CRL.

src/core/tsi/ssl_transport_security_utils.cc

@@ -23,10 +23,6 @@
 #include <openssl/crypto.h>
 #include <openssl/err.h>
 #include <openssl/ssl.h>
-#include <openssl/x509v3.h>
-
-#include "absl/status/status.h"
-#include "absl/status/statusor.h"
 
 #include "src/core/tsi/transport_security_interface.h"
 
@@ -251,66 +247,4 @@ tsi_result SslProtectorUnprotect(const unsigned char* protected_frames_bytes,
   return result;
 }
 
-bool VerifyCrlSignature(X509_CRL* crl, X509* issuer) {
-  if (issuer == nullptr || crl == nullptr) {
-    return false;
-  }
-  EVP_PKEY* ikey = X509_get_pubkey(issuer);
-  if (ikey == nullptr) {
-    // Can't verify signature because we couldn't get the pubkey, fail the
-    // check.
-    EVP_PKEY_free(ikey);
-    return false;
-  }
-  bool ret = X509_CRL_verify(crl, ikey) == 1;
-  EVP_PKEY_free(ikey);
-  return ret;
-}
-
-bool VerifyCrlCertIssuerNamesMatch(X509_CRL* crl, X509* cert) {
-  if (cert == nullptr || crl == nullptr) {
-    return false;
-  }
-  X509_NAME* cert_issuer_name = X509_get_issuer_name(cert);
-  if (cert == nullptr) {
-    return false;
-  }
-  X509_NAME* crl_issuer_name = X509_CRL_get_issuer(crl);
-  if (crl_issuer_name == nullptr) {
-    return false;
-  }
-  return X509_NAME_cmp(cert_issuer_name, crl_issuer_name) == 0;
-}
-
-bool HasCrlSignBit(X509* cert) {
-  if (cert == nullptr) {
-    return false;
-  }
-  // X509_get_key_usage was introduced in 1.1.1
-  // A missing key usage extension means all key usages are valid.
-#if OPENSSL_VERSION_NUMBER < 0x10100000
-  if (!cert->ex_flags & EXFLAG_KUSAGE) {
-    return true;
-  }
-  return cert->ex_kusage & KU_CRL_SIGN;
-#else
-  return (X509_get_key_usage(cert) & KU_CRL_SIGN) != 0;
-#endif  // OPENSSL_VERSION_NUMBER < 0x10100000
-}
-
-absl::StatusOr<std::string> IssuerFromCert(X509* cert) {
-  if (cert == nullptr) {
-    return absl::InvalidArgumentError("cert cannot be null");
-  }
-  X509_NAME* issuer = X509_get_issuer_name(cert);
-  unsigned char* buf = nullptr;
-  int len = i2d_X509_NAME(issuer, &buf);
-  if (len < 0 || buf == nullptr) {
-    return absl::InvalidArgumentError("could not read issuer name from cert");
-  }
-  std::string ret(reinterpret_cast<char const*>(buf), len);
-  OPENSSL_free(buf);
-  return ret;
-}
-
 }  // namespace grpc_core

src/core/tsi/ssl_transport_security_utils.h

@@ -23,8 +23,6 @@
 
 #include <openssl/x509.h>
 
-#include "absl/status/status.h"
-#include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
 
 #include <grpc/grpc_security_constants.h>
@@ -144,22 +142,6 @@ tsi_result SslProtectorUnprotect(const unsigned char* protected_frames_bytes,
                                  unsigned char* unprotected_bytes,
                                  size_t* unprotected_bytes_size);
 
-// Verifies that `crl` was signed by `issuer.
-// return: true if valid, false otherwise.
-bool VerifyCrlSignature(X509_CRL* crl, X509* issuer);
-
-// Verifies the CRL issuer and certificate issuer name match.
-// return: true if equal, false if not.
-bool VerifyCrlCertIssuerNamesMatch(X509_CRL* crl, X509* cert);
-
-// Verifies the certificate in question has the cRLSign bit present.
-// return: true if cRLSign bit is present, false otherwise.
-bool HasCrlSignBit(X509* cert);
-
-// Gets a stable representation of the issuer name from an X509 certificate.
-// return: a std::string of the DER encoding of the X509_NAME issuer name.
-absl::StatusOr<std::string> IssuerFromCert(X509* cert);
-
 }  // namespace grpc_core
 
 #endif  // GRPC_SRC_CORE_TSI_SSL_TRANSPORT_SECURITY_UTILS_H

test/core/security/BUILD

@@ -569,7 +569,6 @@ grpc_cc_test(
     srcs = ["grpc_tls_crl_provider_test.cc"],
     data = [
         "//test/core/tsi/test_creds/crl_data:ca.pem",
-        "//test/core/tsi/test_creds/crl_data:intermediate_ca.pem",
         "//test/core/tsi/test_creds/crl_data/crls:ab06acdd.r0",
         "//test/core/tsi/test_creds/crl_data/crls:b9322cac.r0",
         "//test/core/tsi/test_creds/crl_data/crls:current.crl",
@@ -586,7 +585,6 @@ grpc_cc_test(
         "//test/core/event_engine:event_engine_test_utils",
         "//test/core/event_engine/fuzzing_event_engine",
         "//test/core/event_engine/fuzzing_event_engine:fuzzing_event_engine_proto",
-        "//test/core/tsi:transport_security_test_lib",
         "//test/core/util:fuzz_config_vars_proto",
         "//test/core/util:grpc_test_util",
     ],

test/core/security/grpc_tls_crl_provider_test.cc

@@ -40,15 +40,16 @@
 #include "test/core/event_engine/event_engine_test_utils.h"
 #include "test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h"
 #include "test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.pb.h"
-#include "test/core/tsi/transport_security_test_lib.h"
 #include "test/core/util/test_config.h"
 #include "test/core/util/tls_utils.h"
 
 static constexpr absl::string_view kCrlPath =
     "test/core/tsi/test_creds/crl_data/crls/current.crl";
 static constexpr absl::string_view kCrlName = "current.crl";
-static constexpr absl::string_view kCrlIntermediateIssuerPath =
-    "test/core/tsi/test_creds/crl_data/intermediate_ca.pem";
+static constexpr absl::string_view kCrlIssuer =
+    "/C=AU/ST=Some-State/O=Internet Widgits Pty Ltd/CN=testca";
+static constexpr absl::string_view kCrlIntermediateIssuer =
+    "/CN=intermediatecert.example.com";
 static constexpr absl::string_view kCrlDirectory =
     "test/core/tsi/test_creds/crl_data/crls";
 static constexpr absl::string_view kRootCert =
@@ -87,35 +88,9 @@ class FakeDirectoryReader : public DirectoryReader {
       std::vector<std::string>();
 };
 
-class CrlProviderTest : public ::testing::Test {
+class DirectoryReloaderCrlProviderTest : public ::testing::Test {
  public:
   void SetUp() override {
-    std::string pem_cert = GetFileContents(kRootCert.data());
-    X509* issuer = ReadPemCert(pem_cert);
-    auto base_crl_issuer = IssuerFromCert(issuer);
-    ASSERT_EQ(base_crl_issuer.status(), absl::OkStatus());
-    base_crl_issuer_ = *base_crl_issuer;
-    std::string intermediate_string =
-        GetFileContents(kCrlIntermediateIssuerPath.data());
-    X509* intermediate_issuer = ReadPemCert(intermediate_string);
-    auto intermediate_crl_issuer = IssuerFromCert(intermediate_issuer);
-    ASSERT_EQ(intermediate_crl_issuer.status(), absl::OkStatus());
-    intermediate_crl_issuer_ = *intermediate_crl_issuer;
-    X509_free(issuer);
-    X509_free(intermediate_issuer);
-  }
-
-  void TearDown() override {}
-
- protected:
-  std::string base_crl_issuer_;
-  std::string intermediate_crl_issuer_;
-};
-
-class DirectoryReloaderCrlProviderTest : public CrlProviderTest {
- public:
-  void SetUp() override {
-    CrlProviderTest::SetUp();
     event_engine_ =
         std::make_shared<grpc_event_engine::experimental::FuzzingEventEngine>(
             grpc_event_engine::experimental::FuzzingEventEngine::Options(),
@@ -165,15 +140,15 @@ class DirectoryReloaderCrlProviderTest : public CrlProviderTest {
       event_engine_;
 };
 
-TEST_F(CrlProviderTest, CanParseCrl) {
+TEST(CrlProviderTest, CanParseCrl) {
   std::string crl_string = GetFileContents(kCrlPath.data());
   absl::StatusOr<std::shared_ptr<Crl>> crl = Crl::Parse(crl_string);
   ASSERT_TRUE(crl.ok()) << crl.status();
   ASSERT_NE(*crl, nullptr);
-  EXPECT_EQ((*crl)->Issuer(), base_crl_issuer_);
+  EXPECT_EQ((*crl)->Issuer(), kCrlIssuer);
 }
 
-TEST_F(CrlProviderTest, InvalidFile) {
+TEST(CrlProviderTest, InvalidFile) {
   std::string crl_string = "INVALID CRL FILE";
   absl::StatusOr<std::shared_ptr<Crl>> crl = Crl::Parse(crl_string);
   EXPECT_EQ(crl.status(),
@@ -181,18 +156,18 @@ TEST_F(CrlProviderTest, InvalidFile) {
                 "Conversion from PEM string to X509 CRL failed."));
 }
 
-TEST_F(CrlProviderTest, StaticCrlProviderLookup) {
+TEST(CrlProviderTest, StaticCrlProviderLookup) {
   std::vector<std::string> crl_strings = {GetFileContents(kCrlPath.data())};
   absl::StatusOr<std::shared_ptr<CrlProvider>> provider =
       experimental::CreateStaticCrlProvider(crl_strings);
   ASSERT_TRUE(provider.ok()) << provider.status();
-  CertificateInfoImpl cert(base_crl_issuer_);
+  CertificateInfoImpl cert(kCrlIssuer);
   auto crl = (*provider)->GetCrl(cert);
   ASSERT_NE(crl, nullptr);
-  EXPECT_EQ(crl->Issuer(), base_crl_issuer_);
+  EXPECT_EQ(crl->Issuer(), kCrlIssuer);
 }
 
-TEST_F(CrlProviderTest, StaticCrlProviderLookupIssuerNotFound) {
+TEST(CrlProviderTest, StaticCrlProviderLookupIssuerNotFound) {
   std::vector<std::string> crl_strings = {GetFileContents(kCrlPath.data())};
   absl::StatusOr<std::shared_ptr<CrlProvider>> provider =
       experimental::CreateStaticCrlProvider(crl_strings);
@@ -206,14 +181,14 @@ TEST_F(DirectoryReloaderCrlProviderTest, CrlLookupGood) {
   auto provider =
       CreateCrlProvider(kCrlDirectory, std::chrono::seconds(60), nullptr);
   ASSERT_TRUE(provider.ok()) << provider.status();
-  CertificateInfoImpl cert(base_crl_issuer_);
+  CertificateInfoImpl cert(kCrlIssuer);
   auto crl = (*provider)->GetCrl(cert);
   ASSERT_NE(crl, nullptr);
-  EXPECT_EQ(crl->Issuer(), base_crl_issuer_);
-  CertificateInfoImpl intermediate(intermediate_crl_issuer_);
+  EXPECT_EQ(crl->Issuer(), kCrlIssuer);
+  CertificateInfoImpl intermediate(kCrlIntermediateIssuer);
   auto intermediate_crl = (*provider)->GetCrl(intermediate);
   ASSERT_NE(intermediate_crl, nullptr);
-  EXPECT_EQ(intermediate_crl->Issuer(), intermediate_crl_issuer_);
+  EXPECT_EQ(intermediate_crl->Issuer(), kCrlIntermediateIssuer);
 }
 
 TEST_F(DirectoryReloaderCrlProviderTest, CrlLookupMissingIssuer) {
@@ -229,7 +204,7 @@ TEST_F(DirectoryReloaderCrlProviderTest, ReloadsAndDeletes) {
   const std::chrono::seconds kRefreshDuration(60);
   auto provider = CreateCrlProvider(kRefreshDuration, nullptr);
   ASSERT_TRUE(provider.ok()) << provider.status();
-  CertificateInfoImpl cert(base_crl_issuer_);
+  CertificateInfoImpl cert(kCrlIssuer);
   auto should_be_no_crl = (*provider)->GetCrl(cert);
   ASSERT_EQ(should_be_no_crl, nullptr);
   // Give the provider files to find in the directory
@@ -237,7 +212,7 @@ TEST_F(DirectoryReloaderCrlProviderTest, ReloadsAndDeletes) {
   event_engine_->TickForDuration(kRefreshDuration);
   auto crl = (*provider)->GetCrl(cert);
   ASSERT_NE(crl, nullptr);
-  EXPECT_EQ(crl->Issuer(), base_crl_issuer_);
+  EXPECT_EQ(crl->Issuer(), kCrlIssuer);
   // Now we won't see any files in our directory
   directory_reader_->SetFilesInDirectory({});
   event_engine_->TickForDuration(kRefreshDuration);
@@ -254,10 +229,10 @@ TEST_F(DirectoryReloaderCrlProviderTest, WithCorruption) {
   auto provider =
       CreateCrlProvider(kRefreshDuration, std::move(reload_error_callback));
   ASSERT_TRUE(provider.ok()) << provider.status();
-  CertificateInfoImpl cert(base_crl_issuer_);
+  CertificateInfoImpl cert(kCrlIssuer);
   auto crl = (*provider)->GetCrl(cert);
   ASSERT_NE(crl, nullptr);
-  EXPECT_EQ(crl->Issuer(), base_crl_issuer_);
+  EXPECT_EQ(crl->Issuer(), kCrlIssuer);
   EXPECT_EQ(reload_errors.size(), 0);
   // Point the provider at a non-crl file so loading fails
   // Should result in the CRL Reloader keeping the old CRL data
@@ -265,7 +240,7 @@ TEST_F(DirectoryReloaderCrlProviderTest, WithCorruption) {
   event_engine_->TickForDuration(kRefreshDuration);
   auto crl_post_update = (*provider)->GetCrl(cert);
   ASSERT_NE(crl_post_update, nullptr);
-  EXPECT_EQ(crl_post_update->Issuer(), base_crl_issuer_);
+  EXPECT_EQ(crl_post_update->Issuer(), kCrlIssuer);
   EXPECT_EQ(reload_errors.size(), 1);
 }
 

test/core/tsi/BUILD

@@ -64,23 +64,12 @@ grpc_cc_test(
 grpc_cc_test(
     name = "ssl_transport_security_utils_test",
     srcs = ["ssl_transport_security_utils_test.cc"],
-    data = [
-        "//test/core/tsi/test_creds/crl_data:ca.pem",
-        "//test/core/tsi/test_creds/crl_data:evil_ca.pem",
-        "//test/core/tsi/test_creds/crl_data:intermediate_ca.pem",
-        "//test/core/tsi/test_creds/crl_data:leaf_signed_by_intermediate.pem",
-        "//test/core/tsi/test_creds/crl_data/crls:current.crl",
-        "//test/core/tsi/test_creds/crl_data/crls:intermediate.crl",
-        "//test/core/tsi/test_creds/crl_data/crls:invalid_content.crl",
-        "//test/core/tsi/test_creds/crl_data/crls:invalid_signature.crl",
-    ],
     external_deps = ["gtest"],
     language = "C++",
     tags = ["no_windows"],
     deps = [
         "//:gpr",
         "//:grpc",
-        "//test/core/tsi:transport_security_test_lib",
         "//test/core/util:grpc_test_util",
     ],
 )

test/core/tsi/ssl_transport_security_utils_test.cc

@@ -25,7 +25,6 @@
 #include <openssl/bio.h>
 #include <openssl/crypto.h>
 #include <openssl/ssl.h>
-#include <openssl/x509.h>
 
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
@@ -33,30 +32,13 @@
 #include "absl/strings/str_cat.h"
 #include "absl/strings/string_view.h"
 
-#include "src/core/lib/gprpp/load_file.h"
-#include "src/core/lib/slice/slice.h"
 #include "src/core/tsi/transport_security.h"
 #include "src/core/tsi/transport_security_interface.h"
-#include "test/core/tsi/transport_security_test_lib.h"
 #include "test/core/util/test_config.h"
 
 namespace grpc_core {
 namespace testing {
 
-const char* kValidCrl = "test/core/tsi/test_creds/crl_data/crls/current.crl";
-const char* kCrlIssuer = "test/core/tsi/test_creds/crl_data/ca.pem";
-const char* kModifiedSignature =
-    "test/core/tsi/test_creds/crl_data/crls/invalid_signature.crl";
-const char* kModifiedContent =
-    "test/core/tsi/test_creds/crl_data/crls/invalid_content.crl";
-const char* kIntermediateCrl =
-    "test/core/tsi/test_creds/crl_data/crls/intermediate.crl";
-const char* kIntermediateCrlIssuer =
-    "test/core/tsi/test_creds/crl_data/intermediate_ca.pem";
-const char* kLeafCert =
-    "test/core/tsi/test_creds/crl_data/leaf_signed_by_intermediate.pem";
-const char* kEvilCa = "test/core/tsi/test_creds/crl_data/evil_ca.pem";
-
 using ::testing::ContainerEq;
 using ::testing::NotNull;
 using ::testing::TestWithParam;
@@ -334,8 +316,8 @@ TEST_P(FlowTest,
                                 &protected_output_frames_size),
             tsi_result::TSI_OK);
 
-  // If |GetParam().plaintext_size| is larger than the inner client_buffer
-  // size (kMaxPlaintextBytesPerTlsRecord), then |Protect| will copy up to
+  // If |GetParam().plaintext_size| is larger than the inner client_buffer size
+  // (kMaxPlaintextBytesPerTlsRecord), then |Protect| will copy up to
   // |kMaxPlaintextBytesPerTlsRecord| bytes and output the protected
   // frame. Otherwise we need to manually flush the copied data in order
   // to get the protected frame.
@@ -396,8 +378,8 @@ TEST_P(FlowTest,
                                 &protected_output_frames_size),
             tsi_result::TSI_OK);
 
-  // If |GetParam().plaintext_size| is larger than the inner server_buffer
-  // size (kMaxPlaintextBytesPerTlsRecord), then |Protect| will copy up to
+  // If |GetParam().plaintext_size| is larger than the inner server_buffer size
+  // (kMaxPlaintextBytesPerTlsRecord), then |Protect| will copy up to
   // |kMaxPlaintextBytesPerTlsRecord| bytes and output the protected
   // frame. Otherwise we need to manually flush the copied data in order
   // to get the protected frame.
@@ -447,178 +429,6 @@ INSTANTIATE_TEST_SUITE_P(FrameProtectorUtil, FlowTest,
 
 #endif  // OPENSSL_IS_BORINGSSL
 
-class CrlUtils : public ::testing::Test {
- public:
-  void SetUp() override {
-    absl::StatusOr<Slice> root_crl = LoadFile(kValidCrl, false);
-    ASSERT_EQ(root_crl.status(), absl::OkStatus()) << root_crl.status();
-    root_crl_ = ReadCrl(root_crl->as_string_view());
-    absl::StatusOr<Slice> intermediate_crl = LoadFile(kIntermediateCrl, false);
-    ASSERT_EQ(intermediate_crl.status(), absl::OkStatus())
-        << intermediate_crl.status();
-    intermediate_crl_ = ReadCrl(intermediate_crl->as_string_view());
-    absl::StatusOr<Slice> invalid_signature_crl =
-        LoadFile(kModifiedSignature, false);
-    ASSERT_EQ(invalid_signature_crl.status(), absl::OkStatus())
-        << invalid_signature_crl.status();
-    invalid_signature_crl_ = ReadCrl(invalid_signature_crl->as_string_view());
-
-    absl::StatusOr<Slice> root_ca = LoadFile(kCrlIssuer, false);
-    ASSERT_EQ(root_ca.status(), absl::OkStatus());
-    root_ca_ = ReadPemCert(root_ca->as_string_view());
-    absl::StatusOr<Slice> intermediate_ca =
-        LoadFile(kIntermediateCrlIssuer, false);
-    ASSERT_EQ(intermediate_ca.status(), absl::OkStatus());
-    intermediate_ca_ = ReadPemCert(intermediate_ca->as_string_view());
-    absl::StatusOr<Slice> leaf_cert = LoadFile(kLeafCert, false);
-    ASSERT_EQ(leaf_cert.status(), absl::OkStatus());
-    leaf_cert_ = ReadPemCert(leaf_cert->as_string_view());
-    absl::StatusOr<Slice> evil_ca = LoadFile(kEvilCa, false);
-    ASSERT_EQ(evil_ca.status(), absl::OkStatus());
-    evil_ca_ = ReadPemCert(evil_ca->as_string_view());
-  }
-
-  void TearDown() override {
-    X509_CRL_free(root_crl_);
-    X509_CRL_free(intermediate_crl_);
-    X509_CRL_free(invalid_signature_crl_);
-    X509_free(root_ca_);
-    X509_free(intermediate_ca_);
-    X509_free(leaf_cert_);
-    X509_free(evil_ca_);
-  }
-
- protected:
-  X509_CRL* root_crl_;
-  X509_CRL* intermediate_crl_;
-  X509_CRL* invalid_signature_crl_;
-  X509* root_ca_;
-  X509* intermediate_ca_;
-  X509* leaf_cert_;
-  X509* evil_ca_;
-};
-
-TEST_F(CrlUtils, VerifySignatureValid) {
-  EXPECT_TRUE(VerifyCrlSignature(root_crl_, root_ca_));
-}
-
-TEST_F(CrlUtils, VerifySignatureIntermediateValid) {
-  EXPECT_TRUE(VerifyCrlSignature(intermediate_crl_, intermediate_ca_));
-}
-
-TEST_F(CrlUtils, VerifySignatureModifiedSignature) {
-  EXPECT_FALSE(VerifyCrlSignature(invalid_signature_crl_, root_ca_));
-}
-
-TEST_F(CrlUtils, VerifySignatureModifiedContent) {
-  absl::StatusOr<Slice> crl_slice = LoadFile(kModifiedContent, false);
-  ASSERT_EQ(crl_slice.status(), absl::OkStatus()) << crl_slice.status();
-  X509_CRL* crl = ReadCrl(crl_slice->as_string_view());
-  EXPECT_EQ(crl, nullptr);
-}
-
-TEST_F(CrlUtils, VerifySignatureWrongIssuer) {
-  EXPECT_FALSE(VerifyCrlSignature(root_crl_, intermediate_ca_));
-}
-
-TEST_F(CrlUtils, VerifySignatureWrongIssuer2) {
-  EXPECT_FALSE(VerifyCrlSignature(intermediate_crl_, root_ca_));
-}
-
-TEST_F(CrlUtils, VerifySignatureNullCrl) {
-  EXPECT_FALSE(VerifyCrlSignature(nullptr, root_ca_));
-}
-
-TEST_F(CrlUtils, VerifySignatureNullCert) {
-  EXPECT_FALSE(VerifyCrlSignature(intermediate_crl_, nullptr));
-}
-
-TEST_F(CrlUtils, VerifySignatureNullCrlAndCert) {
-  EXPECT_FALSE(VerifyCrlSignature(nullptr, nullptr));
-}
-
-TEST_F(CrlUtils, VerifyIssuerNamesMatch) {
-  EXPECT_TRUE(VerifyCrlCertIssuerNamesMatch(root_crl_, root_ca_));
-}
-
-TEST_F(CrlUtils, VerifyIssuerNamesDontMatch) {
-  EXPECT_FALSE(VerifyCrlCertIssuerNamesMatch(root_crl_, leaf_cert_));
-}
-
-TEST_F(CrlUtils, DuplicatedIssuerNamePassesButSignatureCheckFails) {
-  // The issuer names will match, but it should fail a signature check
-  EXPECT_TRUE(VerifyCrlCertIssuerNamesMatch(root_crl_, evil_ca_));
-  EXPECT_FALSE(VerifyCrlSignature(root_crl_, evil_ca_));
-}
-
-TEST_F(CrlUtils, VerifyIssuerNameNullCrl) {
-  EXPECT_FALSE(VerifyCrlCertIssuerNamesMatch(nullptr, root_ca_));
-}
-
-TEST_F(CrlUtils, VerifyIssuerNameNullCert) {
-  EXPECT_FALSE(VerifyCrlCertIssuerNamesMatch(intermediate_crl_, nullptr));
-}
-
-TEST_F(CrlUtils, VerifyIssuerNameNullCrlAndCert) {
-  EXPECT_FALSE(VerifyCrlCertIssuerNamesMatch(nullptr, nullptr));
-}
-
-TEST_F(CrlUtils, HasCrlSignBitExists) { EXPECT_TRUE(HasCrlSignBit(root_ca_)); }
-
-TEST_F(CrlUtils, HasCrlSignBitMissing) {
-  EXPECT_FALSE(HasCrlSignBit(leaf_cert_));
-}
-
-TEST_F(CrlUtils, HasCrlSignBitNullCert) {
-  EXPECT_FALSE(HasCrlSignBit(nullptr));
-}
-
-TEST_F(CrlUtils, IssuerFromIntermediateCert) {
-  auto issuer = IssuerFromCert(intermediate_ca_);
-  // Build the known name for comparison
-  unsigned char* buf = nullptr;
-  X509_NAME* expected_issuer_name = X509_NAME_new();
-  ASSERT_TRUE(
-      X509_NAME_add_entry_by_txt(expected_issuer_name, "C", MBSTRING_ASC,
-                                 (const unsigned char*)"AU", -1, -1, 0));
-  ASSERT_TRUE(X509_NAME_add_entry_by_txt(
-      expected_issuer_name, "ST", MBSTRING_ASC,
-      (const unsigned char*)"Some-State", -1, -1, 0));
-  ASSERT_TRUE(X509_NAME_add_entry_by_txt(
-      expected_issuer_name, "O", MBSTRING_ASC,
-      (const unsigned char*)"Internet Widgits Pty Ltd", -1, -1, 0));
-  ASSERT_TRUE(
-      X509_NAME_add_entry_by_txt(expected_issuer_name, "CN", MBSTRING_ASC,
-                                 (const unsigned char*)"testca", -1, -1, 0));
-  int len = i2d_X509_NAME(expected_issuer_name, &buf);
-  std::string expected_issuer_name_der(reinterpret_cast<char const*>(buf), len);
-  OPENSSL_free(buf);
-  X509_NAME_free(expected_issuer_name);
-  ASSERT_EQ(issuer.status(), absl::OkStatus());
-  EXPECT_EQ(*issuer, expected_issuer_name_der);
-}
-
-TEST_F(CrlUtils, IssuerFromLeaf) {
-  auto issuer = IssuerFromCert(leaf_cert_);
-  // Build the known name for comparison
-  unsigned char* buf = nullptr;
-  X509_NAME* expected_issuer_name = X509_NAME_new();
-  ASSERT_TRUE(X509_NAME_add_entry_by_txt(
-      expected_issuer_name, "CN", MBSTRING_ASC,
-      (const unsigned char*)"intermediatecert.example.com", -1, -1, 0));
-  int len = i2d_X509_NAME(expected_issuer_name, &buf);
-  std::string expected_issuer_name_der(reinterpret_cast<char const*>(buf), len);
-  OPENSSL_free(buf);
-  X509_NAME_free(expected_issuer_name);
-  ASSERT_EQ(issuer.status(), absl::OkStatus());
-  EXPECT_EQ(*issuer, expected_issuer_name_der);
-}
-
-TEST_F(CrlUtils, IssuerFromCertNull) {
-  auto issuer = IssuerFromCert(nullptr);
-  EXPECT_EQ(issuer.status().code(), absl::StatusCode::kInvalidArgument);
-}
-
 }  // namespace testing
 }  // namespace grpc_core
 

test/core/tsi/test_creds/crl_data/BUILD

@@ -25,5 +25,4 @@ exports_files([
     "leaf_and_intermediate_chain.pem",
     "intermediate_ca.key",
     "intermediate_ca.pem",
-    "evil_ca.pem",
 ])

test/core/tsi/test_creds/crl_data/README

@@ -46,11 +46,6 @@ Generate a chain with a leaf cert signed by an intermediate CA and revoke the in
 
 Run `intermediate_gen.sh` from the `test/core/tsi/test_creds/crl_data` directory
 
-Generate a CA with the same issuer name but a different public key than the base CA
-----------------------------------------------------------------------------
-
-Run `evil_ca_gen.sh` from the `test/core/tsi/test_creds/crl_data` directory
-
 
 Clean up:
 ---------

test/core/tsi/test_creds/crl_data/crls/BUILD

@@ -19,6 +19,4 @@ exports_files([
     "b9322cac.r0",
     "current.crl",
     "intermediate.crl",
-    "invalid_signature.crl",
-    "invalid_content.crl",
 ])

test/core/tsi/test_creds/crl_data/crls/invalid_content.crl

@@ -1,15 +0,0 @@
------BEGIN X509 CRL-----
-AIICUDCCATgCAQEwDQYJKoZIhvcNAQELBQAwVjELMAkGA1UEBhMCQVUxEzARBgNV
-BAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0
-ZDEPMA0GA1UEAwwGdGVzdGNhFw0yMzAzMDMxODA2NDNaFw0zMzAyMjgxODA2NDNa
-MIGcMCUCFEpMyQOrk+uXDu20PhHwDJeua83mFw0yMzAzMDMxNjU5NTNaMCUCFEpM
-yQOrk+uXDu20PhHwDJeua83nFw0yMzAzMDMxNzMxNDBaMCUCFEpMyQOrk+uXDu20
-PhHwDJeua83xFw0yMzAzMDMxODA2NDNaMCUCFFIgumScY9chZ0u8tUhjsOUh38hB
-Fw0yMjAyMDQyMjExMTFaoA8wDTALBgNVHRQEBAICEAgwDQYJKoZIhvcNAQELBQAD
-ggEBADohIZwm/gWLIc2yFJJbKzkdRmOq1s/MqnJxi5NutNumXTIPrZJqGzk8O4U6
-VasicIB2YD0o3arzUxCDyHv7VyJI7SVS0lqlmOxoOEOv2+CB6MxAOdKItkzbVVxu
-0erx5HcKAGa7ZIAeekX1F1DcAgpN5Gt5uGhkMw3ObTCpEFRw+ZKET3WFQ6bG4AJ6
-GwOnNYG1LjaNigxG/k4K7A+grs/XnsNcpULbCROl7Qw4kyf1esrjS9utEO0YQQz4
-LgBTPZzQHlsirmxp+e5WR8LiDsKmbmAaBL+gV1Bkjj73c4pNJvoV/V1Ubdv0LCvH
-DjrJtp10F0RGMRm6m9OuZYUSFzs=
------END X509 CRL-----

test/core/tsi/test_creds/crl_data/crls/invalid_signature.crl

@@ -1,15 +0,0 @@
------BEGIN X509 CRL-----
-MIICUDCCATgCAQEwDQYJKoZIhvcNAQELBQAwVjELMAkGA1UEBhMCQVUxEzARBgNV
-BAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0
-ZDEPMA0GA1UEAwwGdGVzdGNhFw0yMzAzMDMxODA2NDNaFw0zMzAyMjgxODA2NDNa
-MIGcMCUCFEpMyQOrk+uXDu20PhHwDJeua83mFw0yMzAzMDMxNjU5NTNaMCUCFEpM
-yQOrk+uXDu20PhHwDJeua83nFw0yMzAzMDMxNzMxNDBaMCUCFEpMyQOrk+uXDu20
-PhHwDJeua83xFw0yMzAzMDMxODA2NDNaMCUCFFIgumScY9chZ0u8tUhjsOUh38hB
-Fw0yMjAyMDQyMjExMTFaoA8wDTALBgNVHRQEBAICEAgwDQYJKoZIhvcNAQELBQAD
-ggEBADohIZwm/gWLIc2yFJJbKzkdRmOq1s/MqnJxi5NutNumXTIPrZJqGzk8O4U6
-VasicIB2YD0o3arzUxCDyHv7VyJI7SVS0lqlmOxoOEOv2+CB6MxAOdKItkzbVVxu
-0erx5HcKAGa7ZIAeekX1F1DcAgpN5Gt5uGhkMw3ObTCpEFRw+ZKET3WFQ6bG4AJ6
-GwOnNYG1LjaNigxG/k4K7A+grs/XnsNcpULbCROl7Qw4kyf1esrjS9utEO0YQQz4
-LgBTPZzQHlsirmxp+e5WR8LiDsKmbmAaBL+gV1Bkjj73c4pNJvoV/V1Ubdv0LCvH
-DjrJtp10F0RGMRm6m9OuZYUSFza=
------END X509 CRL-----

test/core/tsi/test_creds/crl_data/evil_ca.key

@@ -1,28 +0,0 @@
------BEGIN PRIVATE KEY-----
-MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC7RwHo8bWaioeI
-oqq4qRkiRfqAl/XlaRCyygkMtkjuOy0LA42+LFXXNvDD8eVvVd3615Qopm0XzABd
-iz2QiJBZH9qvvmZFg7vG4rbNMCHIN+0YYIOp5tJuyBUVhZ+/f/jZ+LoJeZgTRngQ
-tMUmhs7kn4ttT+DC7ZHKhPf5vUokSPG4N2tBx21y2BzRup36q09vfvZeVEe5YxAM
-KGWEOcCY/S5vTVeEJCqP2OfMmskIHq2cYWr6ZJzBpdhJXX6rTDWYlCzX49mzPrn6
-povhA/bENv9Gy1OHqPKt+EWEJCaurerkFwF74OG9zp/jCKZJTVkyxnCYjT2rYiDX
-gWvNwdeHAgMBAAECggEADyya44Mzj0Y6jXV8tsIA0YLxCrAFZ7q3ydIj9z3ih+cP
-PcK3yUPHYCJJUjR3PipWIP03Dy949xd7pMNjpXfjQPgbRz0lWpboxUiDvk7FlfcD
-b4O2d12cCbI4Px+uHh1M48B1tnnTOtCYFDvJc6yITARUuZ03cs6UDwrvcB1dygsO
-2sZLUOkWQb2DCMq86bxmkHvjuh3gj/CMTJv0Kprlo3YcKNgCwiNygEzlusyIcwpf
-dU/SNoWcxY+F0F6wFC0uj75wWqDB6bmfCpY8Bb3Ey7TgWDTWjsB/NQsWbSxZ9o5i
-qjQ6WSLKpLLLB/8dXxhk3Nz9tfonavBpLB+4fNpFFQKBgQDi61A3/U88iEo+sxMm
-L3i0OS9g/mAnYQ7zYjq42eVyDTfa+eBck1Jmp1KEblfy7Eo3iyApNFoIzFz8va8N
-tPNFK/K4mrf1aiFOk0SnvCstW8SBS99hBHXqrMnXrRh+L/OafM4sj88P4RbZxcIs
-9RNiDIqcXAPDVU5aHIhs7CFzYwKBgQDTRyOR9PoTQnu0HV0ODDNzmP1eRWrXZ62N
-khe9bm0TIG25Q1wsoR6MT5fxZlTe62FH7A5QgEheRtMctr+XGC2H+3N3MUxsTy37
-knPFiDl6Gs5DqKroewiDNbkziMOgctG/z6ORPiGghTRsn6y5dBaMstfvgip8fj5z
-ytzgSfiujQKBgHZraOSfK++iDGTmHRMraOlcgm4ysck7LIs08wIurD+1yDVde4m0
-VCdAIJ792qXqS9zqnPED4gx/YfN/pdAYY2/wvG08SM4pAZK45fZHC51TK5xyFPPT
-WRoL7BXCvmpz6cPwZ8P3lI5r3/nr6yZ9Cw17EAcDOe+BIC+EfmmhXN+TAoGBAIp0
-oDbSV9+vPen3JDhEfqNOqxvQWgf3haC1EKGvcAOMyNsT7Z/BpodE0cn8ybmcfw/m
-/ip7JvHBcC/tAvk9evkWK8D8qZyA9x1aCEx2zVPbpThpnDbmCdoSpt/CzJClLheJ
-NyPDl73eDVDyAvs1vGFQAnqOztDu2nZ/huflEfcxAoGAbLUQV5PjqJrsIosEMXsv
-qOzQZ5BBEk/jo9zqYSNXWVs0I9Invj5iAYewoM5qn9DFQ3q3O/mPHxF6HT7JHfjn
-T8wdOTQk5L1yaaSFsiti3C3AQ2zShT1k6m3V+mf0iWJw878LCURQQFNIHu7zVdXy
-4xwQpVw2CN7iufRYN7kOcDo=
------END PRIVATE KEY-----

test/core/tsi/test_creds/crl_data/evil_ca.pem

@@ -1,21 +0,0 @@
------BEGIN CERTIFICATE-----
-MIIDeTCCAmGgAwIBAgIUULA9nt1NB3W1i4RevrKeRQQLkaIwDQYJKoZIhvcNAQEL
-BQAwVjELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
-GEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDEPMA0GA1UEAwwGdGVzdGNhMB4XDTI0
-MDEyMjIxNDAyMFoXDTM0MDExOTIxNDAyMFowVjELMAkGA1UEBhMCQVUxEzARBgNV
-BAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0
-ZDEPMA0GA1UEAwwGdGVzdGNhMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKC
-AQEAu0cB6PG1moqHiKKquKkZIkX6gJf15WkQssoJDLZI7jstCwONvixV1zbww/Hl
-b1Xd+teUKKZtF8wAXYs9kIiQWR/ar75mRYO7xuK2zTAhyDftGGCDqebSbsgVFYWf
-v3/42fi6CXmYE0Z4ELTFJobO5J+LbU/gwu2RyoT3+b1KJEjxuDdrQcdtctgc0bqd
-+qtPb372XlRHuWMQDChlhDnAmP0ub01XhCQqj9jnzJrJCB6tnGFq+mScwaXYSV1+
-q0w1mJQs1+PZsz65+qaL4QP2xDb/RstTh6jyrfhFhCQmrq3q5BcBe+Dhvc6f4wim
-SU1ZMsZwmI09q2Ig14FrzcHXhwIDAQABoz8wPTAMBgNVHRMEBTADAQH/MA4GA1Ud
-DwEB/wQEAwIBBjAdBgNVHQ4EFgQUjcQvfJ6kAUgljgToPpQ0DmCW0Q8wDQYJKoZI
-hvcNAQELBQADggEBALLNhOYqlhOcCsTD1SPfm9MAjfpV1EjSjDCpIfwCk5gI2CUX
-g7MyUzn2gQJUiYx74BKmjv6W/sLzNxqR0wZQUr4d/7HX+Lm0xCCYdIUELEM8lZ30
-maBJ599cQnLXDB1ZFEekj3DMM6jL7OQnBaDs5jW4GcDcuwd5cgXfgIaZVjBVJ11Y
-CFAhIuh5CM8xhqxWYWY+h0VLU64s8WCNrBEy1OU5KpQRfpd4cvpoWn7E1SfhK1Iq
-Bp+1k4oDBpGGw4NLXI3i1aU8x1+KoXxNRg5dOED0OLgppvaWB2yIpqBlcZDaNpq4
-P+WFGBiSUpWU5yYwCDvQAgTWtWkmyflVwslHaGs=
------END CERTIFICATE-----

test/core/tsi/test_creds/crl_data/evil_ca_gen.sh

@@ -1,18 +0,0 @@
-#!/bin/bash
-# Copyright 2024 gRPC authors.
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-# Generates a CA with the same issuer name as the good CA in this directory
-openssl req -x509 -new -newkey rsa:2048 -nodes -keyout evil_ca.key -out evil_ca.pem \
-  -config ca-openssl.cnf -days 3650 -extensions v3_req

test/core/tsi/transport_security_test_lib.cc

@@ -762,27 +762,3 @@ std::string GenerateSelfSignedCertificate(
   BN_free(n);
   return pem;
 }
-
-X509* ReadPemCert(absl::string_view pem_cert) {
-  BIO* cert_bio =
-      BIO_new_mem_buf(pem_cert.data(), static_cast<int>(pem_cert.size()));
-  // Errors on BIO
-  if (cert_bio == nullptr) {
-    return nullptr;
-  }
-  X509* cert = PEM_read_bio_X509(cert_bio, nullptr, nullptr, nullptr);
-  BIO_free(cert_bio);
-  return cert;
-}
-
-X509_CRL* ReadCrl(absl::string_view crl_pem) {
-  BIO* crl_bio =
-      BIO_new_mem_buf(crl_pem.data(), static_cast<int>(crl_pem.size()));
-  // Errors on BIO
-  if (crl_bio == nullptr) {
-    return nullptr;
-  }
-  X509_CRL* crl = PEM_read_bio_X509_CRL(crl_bio, nullptr, nullptr, nullptr);
-  BIO_free(crl_bio);
-  return crl;
-}

test/core/tsi/transport_security_test_lib.h

@@ -19,8 +19,6 @@
 #ifndef GRPC_TEST_CORE_TSI_TRANSPORT_SECURITY_TEST_LIB_H
 #define GRPC_TEST_CORE_TSI_TRANSPORT_SECURITY_TEST_LIB_H
 
-#include <openssl/x509v3.h>
-
 #include <grpc/support/sync.h>
 
 #include "src/core/tsi/transport_security_interface.h"
@@ -240,10 +238,4 @@ struct SelfSignedCertificateOptions {
 std::string GenerateSelfSignedCertificate(
     const SelfSignedCertificateOptions& options);
 
-// Returns the OpenSSL representation of a PEM cert.
-X509* ReadPemCert(absl::string_view pem_cert);
-
-// Returns the OpenSSL representation of a CRL.
-X509_CRL* ReadCrl(absl::string_view crl_pem);
-
 #endif  // GRPC_TEST_CORE_TSI_TRANSPORT_SECURITY_TEST_LIB_H