[Security - Revocation] Refactor how CRLs are checked internally (#36031)

This PR changes how CRLs are handled purely internally. After discussing with davidben@, there are various problems with the `X509_STORE_set_get_crl` API and we shouldn't use it. This change keeps the behavior and external API the same, but instead of bulk pushing CRL information into OpenSSL, we instead iterate through the built chain and check each certificate for revocation, as well as doing the CRL validation ourselves.

Closes #36031

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36031 from gtcooke94:CrlInternalRefactor 5f4c816648
PiperOrigin-RevId: 615139682

src/core/tsi/ssl_transport_security.cc

@@ -72,6 +72,7 @@
 #define TSI_SSL_MAX_PROTECTED_FRAME_SIZE_UPPER_BOUND 16384
 #define TSI_SSL_MAX_PROTECTED_FRAME_SIZE_LOWER_BOUND 1024
 #define TSI_SSL_HANDSHAKER_OUTGOING_BUFFER_INITIAL_SIZE 1024
+const size_t kMaxChainLength = 100;
 
 // Putting a macro like this and littering the source file with #if is really
 // bad practice.
@@ -911,13 +912,7 @@ static int NullVerifyCallback(X509_STORE_CTX* /*ctx*/, void* /*arg*/) {
 }
 
 static int RootCertExtractCallback(X509_STORE_CTX* ctx, void* /*arg*/) {
-  int ret = X509_verify_cert(ctx);
-  if (ret <= 0) {
-    // Verification failed. We shouldn't expect to have a verified chain, so
-    // there is no need to attempt to extract the root cert from it.
-    return ret;
-  }
-
+  int ret = 1;
   // Verification was successful. Get the verified chain from the X509_STORE_CTX
   // and put the root on the SSL object so that we have access to it when
   // populating the tsi_peer. On error extracting the root, we return success
@@ -977,37 +972,42 @@ static int RootCertExtractCallback(X509_STORE_CTX* ctx, void* /*arg*/) {
   return ret;
 }
 
-// X509_STORE_set_get_crl() sets the function to get the crl for a given
-// certificate x. When found, the crl must be assigned to *crl. This function
-// must return 0 on failure and 1 on success. If no function to get the issuer
-// is provided, the internal default function will be used instead.
-static int GetCrlFromProvider(X509_STORE_CTX* ctx, X509_CRL** crl_out,
-                              X509* cert) {
+static grpc_core::experimental::CrlProvider* GetCrlProvider(
+    X509_STORE_CTX* ctx) {
   ERR_clear_error();
   int ssl_index = SSL_get_ex_data_X509_STORE_CTX_idx();
   if (ssl_index < 0) {
     char err_str[256];
     ERR_error_string_n(ERR_get_error(), err_str, sizeof(err_str));
-    gpr_log(GPR_ERROR,
+    gpr_log(GPR_INFO,
             "error getting the SSL index from the X509_STORE_CTX while looking "
             "up Crl: %s",
             err_str);
-    return 0;
+    return nullptr;
   }
   SSL* ssl = static_cast<SSL*>(X509_STORE_CTX_get_ex_data(ctx, ssl_index));
   if (ssl == nullptr) {
-    gpr_log(GPR_ERROR,
+    gpr_log(GPR_INFO,
             "error while fetching from CrlProvider. SSL object is null");
-    return 0;
+    return nullptr;
   }
   SSL_CTX* ssl_ctx = SSL_get_SSL_CTX(ssl);
   auto* provider = static_cast<grpc_core::experimental::CrlProvider*>(
       SSL_CTX_get_ex_data(ssl_ctx, g_ssl_ctx_ex_crl_provider_index));
+  return provider;
+}
 
+// If a CRL is returned, the caller is the owner of the CRL and must make sure
+// it is freed.
+static absl::StatusOr<X509_CRL*> GetCrlFromProvider(
+    grpc_core::experimental::CrlProvider* provider, X509* cert) {
+  if (provider == nullptr) {
+    return absl::InvalidArgumentError("CrlProvider is null.");
+  }
   absl::StatusOr<std::string> issuer_name = grpc_core::IssuerFromCert(cert);
   if (!issuer_name.ok()) {
     gpr_log(GPR_INFO, "Could not get certificate issuer name");
-    return 0;
+    return absl::InvalidArgumentError(issuer_name.status().message());
   }
   absl::StatusOr<std::string> akid = grpc_core::AkidFromCertificate(cert);
   std::string akid_to_use;
@@ -1026,28 +1026,142 @@ static int GetCrlFromProvider(X509_STORE_CTX* ctx, X509_CRL** crl_out,
   // and behave how we want for a missing CRL.
   // It is important to treat missing CRLs and empty CRLs differently.
   if (internal_crl == nullptr) {
-    return 0;
+    return absl::NotFoundError("Could not find Crl related to certificate.");
   }
   X509_CRL* crl =
       std::static_pointer_cast<grpc_core::experimental::CrlImpl>(internal_crl)
           ->crl();
 
-  X509_CRL* copy = X509_CRL_dup(crl);
-  *crl_out = copy;
+  return X509_CRL_dup(crl);
+}
+
+// Perform the validation checks in RFC5280 6.3.3 to ensure the given CRL is
+// valid
+// returns true if the Crl is valid, false otherwise
+static bool ValidateCrl(X509* cert, X509* issuer, X509_CRL* crl) {
+  bool valid = true;
+  // RFC5280 6.3.3
+  // 6.3.3a we do not support distribution points
+  // 6.3.3b verify issuer and scope
+  valid = grpc_core::VerifyCrlCertIssuerNamesMatch(crl, cert);
+  if (!valid) {
+    return valid;
+  }
+  valid = grpc_core::HasCrlSignBit(issuer);
+  if (!valid) {
+    return valid;
+  }
+  // 6.3.3c Not supporting deltas
+  // 6.3.3d Not supporting reasons masks
+  // 6.3.3e Not supporting reasons masks
+  // 6.3.3f We only support direct CRLs so these paths are by definition the
+  // same.
+  // 6.3.3g Verify CRL Signature
+  valid = grpc_core::VerifyCrlSignature(crl, issuer);
+  return valid;
+}
+
+// Check if a given certificate is revoked
+// Returns 1 if the certificate is not revoked, 0 if the certificate is revoked
+static int CheckCertRevocation(grpc_core::experimental::CrlProvider* provider,
+                               X509* cert, X509* issuer) {
+  auto crl = GetCrlFromProvider(provider, cert);
+  // Not finding a CRL is a specific behavior. Per RFC5280, not having a CRL to
+  // check for a given certificate means that we cannot know for certain if the
+  // status is Revoked or Unrevoked and instead is Undetermined. How a user
+  // handles an Undetermined CRL is up to them. We use absl::IsNotFound as an
+  // analogue for not finding the Crl from the provider, thus the certificate in
+  // question is Undetermined.
+  if (absl::IsNotFound(crl.status())) {
+    // TODO(gtcooke94) knob for undetermined being revoked or unrevoked. By
+    // default, unrevoked.
+    return 1;
+  } else if (!crl.ok()) {
+    // This is an unexpected error, return false
+    return 0;
+  }
+  // Validate the crl
+  // RFC5280 6.3.3(a-i)
+  if (!ValidateCrl(cert, issuer, *crl)) {
+    X509_CRL_free(*crl);
+    return 0;
+  }
+
+  // RFC5280 6.3.3j Actually check revocation
+  // Look for serial number of certificate in CRL  X509_REVOKED* rev =
+  // nullptr;
+  X509_REVOKED* rev;
+  if (X509_CRL_get0_by_cert(*crl, &rev, cert)) {
+    // cert is revoked
+    X509_CRL_free(*crl);
+    return 0;
+  }
+  // The certificate is not revoked
+  // RFC5280k - Not supported
+  // RFC5280l - Not supported
+  X509_CRL_free(*crl);
   return 1;
 }
 
-// When using CRL Providers, this function used to override the default
-// `check_crl` function in OpenSSL using `X509_STORE_set_check_crl`.
-// CrlProviders put the onus on the users to provide the CRLs that they want to
-// provide, and because we override default CRL fetching behavior, we can expect
-// some of these verification checks to fails for custom CRL providers as well.
-// Thus, we need a passthrough to indicate to OpenSSL that we've provided a CRL
-// and we are good with it.
-static int CheckCrlPassthrough(X509_STORE_CTX* /*ctx*/, X509_CRL* /*crl*/) {
+// Checks each certificate in the chain for revocation
+// returns 0 if any cert in the chain is revoked, 1 otherwise.
+static int CheckChainRevocation(
+    X509_STORE_CTX* ctx, grpc_core::experimental::CrlProvider* provider) {
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+  STACK_OF(X509)* chain = X509_STORE_CTX_get0_chain(ctx);
+#else
+  STACK_OF(X509)* chain = X509_STORE_CTX_get_chain(ctx);
+#endif
+  if (chain == nullptr) {
+    return 0;
+  }
+  // BoringSSL returns a size_t (unsigned), while OpenSSL returns an int
+  // (signed). In OpenSSL, a -1 can indicate a problem. By forcing it into a
+  // size_t, a -1 return will result in the chain_length being a very large
+  // number, so it will still fail this check because that very large number
+  // will be >= kMaxChainLength
+  size_t chain_length = sk_X509_num(chain);
+  if (chain_length > kMaxChainLength || chain_length == 0) {
+    return 0;
+  }
+  // Loop to < chain_length - 1 because the last cert is the trust anchor/root
+  // which cannot be revoked
+  for (size_t i = 0; i < chain_length - 1; i++) {
+    X509* cert = sk_X509_value(chain, i);
+    X509* issuer = sk_X509_value(chain, i + 1);
+    int ret = CheckCertRevocation(provider, cert, issuer);
+    if (ret != 1) {
+      return ret;
+    }
+  }
   return 1;
 }
 
+// The custom verification function to set in OpenSSL using
+// X509_set_cert_verify_callback. This calls the standard OpenSSL procedure
+// (X509_verify_cert), then also extracts the root certificate in the built
+// chain and does revocation checks when a user has configured CrlProviders.
+// returns 1 on success, indicating a trusted chain to a root of trust was
+// found, 0 if a trusted chain could not be built.
+static int CustomVerificationFunction(X509_STORE_CTX* ctx, void* arg) {
+  int ret = X509_verify_cert(ctx);
+  if (ret <= 0) {
+    // Verification failed. We shouldn't expect to have a verified chain, so
+    // there is no need to attempt to extract the root cert from it, check for
+    // revocation, or check anything else.
+    return ret;
+  }
+  grpc_core::experimental::CrlProvider* provider = GetCrlProvider(ctx);
+  if (provider != nullptr) {
+    ret = CheckChainRevocation(ctx, provider);
+  }
+  if (ret <= 0) {
+    // Something has failed return the failure
+    return ret;
+  }
+  return RootCertExtractCallback(ctx, arg);
+}
+
 // Sets the min and max TLS version of |ssl_context| to |min_tls_version| and
 // |max_tls_version|, respectively. Calling this method is a no-op when using
 // OpenSSL versions < 1.1.
@@ -1069,9 +1183,9 @@ static tsi_result tsi_set_min_and_max_tls_versions(
       SSL_CTX_set_min_proto_version(ssl_context, TLS1_2_VERSION);
       break;
 #if defined(TLS1_3_VERSION)
-    // If the library does not support TLS 1.3 and the caller requests a minimum
-    // of TLS 1.3, then return an error because the caller's request cannot be
-    // satisfied.
+    // If the library does not support TLS 1.3 and the caller requests a
+    // minimum of TLS 1.3, then return an error because the caller's request
+    // cannot be satisfied.
     case tsi_tls_version::TSI_TLS1_3:
       SSL_CTX_set_min_proto_version(ssl_context, TLS1_3_VERSION);
       break;
@@ -1131,6 +1245,12 @@ tsi_ssl_root_certs_store* tsi_ssl_root_certs_store_create(
     gpr_free(root_store);
     return nullptr;
   }
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+  X509_VERIFY_PARAM* param = X509_STORE_get0_param(root_store->store);
+#else
+  X509_VERIFY_PARAM* param = root_store->store->param;
+#endif
+  X509_VERIFY_PARAM_set_depth(param, kMaxChainLength);
   return root_store;
 }
 
@@ -1586,8 +1706,8 @@ static tsi_result ssl_bytes_remaining(tsi_ssl_handshaker* impl,
   *bytes_remaining = static_cast<uint8_t*>(gpr_malloc(bytes_in_ssl));
   int bytes_read = BIO_read(SSL_get_rbio(impl->ssl), *bytes_remaining,
                             static_cast<int>(bytes_in_ssl));
-  // If an unexpected number of bytes were read, return an error status and free
-  // all of the bytes that were read.
+  // If an unexpected number of bytes were read, return an error status and
+  // free all of the bytes that were read.
   if (bytes_read < 0 || static_cast<size_t>(bytes_read) != bytes_in_ssl) {
     gpr_log(GPR_ERROR,
             "Failed to read the expected number of bytes from SSL object.");
@@ -1662,16 +1782,16 @@ static tsi_result ssl_handshaker_next(tsi_handshaker* self,
           impl, remaining_bytes_to_write_to_openssl, &bytes_written_to_openssl,
           error);
       // As long as the BIO is full, drive the SSL handshake to consume bytes
-      // from the BIO. If the SSL handshake returns any bytes, write them to the
-      // peer.
+      // from the BIO. If the SSL handshake returns any bytes, write them to
+      // the peer.
       while (status == TSI_DRAIN_BUFFER) {
         status =
             ssl_handshaker_write_output_buffer(self, &bytes_written, error);
         if (status != TSI_OK) return status;
         status = ssl_handshaker_do_handshake(impl, error);
       }
-      // Move the pointer to the first byte not yet successfully written to the
-      // BIO.
+      // Move the pointer to the first byte not yet successfully written to
+      // the BIO.
       remaining_bytes_to_write_to_openssl_size -= bytes_written_to_openssl;
       remaining_bytes_to_write_to_openssl += bytes_written_to_openssl;
     }
@@ -1687,9 +1807,9 @@ static tsi_result ssl_handshaker_next(tsi_handshaker* self,
     *handshaker_result = nullptr;
   } else {
     // Any bytes that remain in |impl->ssl|'s read BIO after the handshake is
-    // complete must be extracted and set to the unused bytes of the handshaker
-    // result. This indicates to the gRPC stack that there are bytes from the
-    // peer that must be processed.
+    // complete must be extracted and set to the unused bytes of the
+    // handshaker result. This indicates to the gRPC stack that there are
+    // bytes from the peer that must be processed.
     unsigned char* unused_bytes = nullptr;
     size_t unused_bytes_size = 0;
     status =
@@ -1704,8 +1824,8 @@ static tsi_result ssl_handshaker_next(tsi_handshaker* self,
     status = ssl_handshaker_result_create(impl, unused_bytes, unused_bytes_size,
                                           handshaker_result, error);
     if (status == TSI_OK) {
-      // Indicates that the handshake has completed and that a handshaker_result
-      // has been created.
+      // Indicates that the handshake has completed and that a
+      // handshaker_result has been created.
       self->handshaker_result_created = true;
     }
   }
@@ -2152,6 +2272,15 @@ tsi_result tsi_create_ssl_client_handshaker_factory_with_options(
       result = ssl_ctx_load_verification_certs(
           ssl_context, options->pem_root_certs, strlen(options->pem_root_certs),
           nullptr);
+      X509_STORE* cert_store = SSL_CTX_get_cert_store(ssl_context);
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+      X509_VERIFY_PARAM* param = X509_STORE_get0_param(cert_store);
+
+#else
+      X509_VERIFY_PARAM* param = cert_store->param;
+#endif
+
+      X509_VERIFY_PARAM_set_depth(param, kMaxChainLength);
       if (result != TSI_OK) {
         gpr_log(GPR_ERROR, "Cannot load server root certificates.");
         break;
@@ -2189,21 +2318,13 @@ tsi_result tsi_create_ssl_client_handshaker_factory_with_options(
   if (options->skip_server_certificate_verification) {
     SSL_CTX_set_cert_verify_callback(ssl_context, NullVerifyCallback, nullptr);
   } else {
-    SSL_CTX_set_cert_verify_callback(ssl_context, RootCertExtractCallback,
+    SSL_CTX_set_cert_verify_callback(ssl_context, CustomVerificationFunction,
                                      nullptr);
   }
-
 #if OPENSSL_VERSION_NUMBER >= 0x10100000 && !defined(LIBRESSL_VERSION_NUMBER)
   if (options->crl_provider != nullptr) {
     SSL_CTX_set_ex_data(impl->ssl_context, g_ssl_ctx_ex_crl_provider_index,
                         options->crl_provider.get());
-    X509_STORE* cert_store = SSL_CTX_get_cert_store(impl->ssl_context);
-    X509_STORE_set_get_crl(cert_store, GetCrlFromProvider);
-    X509_STORE_set_check_crl(cert_store, CheckCrlPassthrough);
-    X509_STORE_set_verify_cb(cert_store, verify_cb);
-    X509_VERIFY_PARAM* param = X509_STORE_get0_param(cert_store);
-    X509_VERIFY_PARAM_set_flags(
-        param, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL);
   } else if (options->crl_directory != nullptr &&
              strcmp(options->crl_directory, "") != 0) {
     X509_STORE* cert_store = SSL_CTX_get_cert_store(ssl_context);
@@ -2379,7 +2500,7 @@ tsi_result tsi_create_ssl_server_handshaker_factory_with_options(
         case TSI_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY:
           SSL_CTX_set_verify(impl->ssl_contexts[i], SSL_VERIFY_PEER, nullptr);
           SSL_CTX_set_cert_verify_callback(impl->ssl_contexts[i],
-                                           RootCertExtractCallback, nullptr);
+                                           CustomVerificationFunction, nullptr);
           break;
         case TSI_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_BUT_DONT_VERIFY:
           SSL_CTX_set_verify(impl->ssl_contexts[i],
@@ -2393,7 +2514,7 @@ tsi_result tsi_create_ssl_server_handshaker_factory_with_options(
                              SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
                              nullptr);
           SSL_CTX_set_cert_verify_callback(impl->ssl_contexts[i],
-                                           RootCertExtractCallback, nullptr);
+                                           CustomVerificationFunction, nullptr);
           break;
       }
 
@@ -2402,13 +2523,6 @@ tsi_result tsi_create_ssl_server_handshaker_factory_with_options(
         SSL_CTX_set_ex_data(impl->ssl_contexts[i],
                             g_ssl_ctx_ex_crl_provider_index,
                             options->crl_provider.get());
-        X509_STORE* cert_store = SSL_CTX_get_cert_store(impl->ssl_contexts[i]);
-        X509_STORE_set_get_crl(cert_store, GetCrlFromProvider);
-        X509_STORE_set_check_crl(cert_store, CheckCrlPassthrough);
-        X509_STORE_set_verify_cb(cert_store, verify_cb);
-        X509_VERIFY_PARAM* param = X509_STORE_get0_param(cert_store);
-        X509_VERIFY_PARAM_set_flags(
-            param, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL);
       } else if (options->crl_directory != nullptr &&
                  strcmp(options->crl_directory, "") != 0) {
         X509_STORE* cert_store = SSL_CTX_get_cert_store(impl->ssl_contexts[i]);

test/core/tsi/BUILD

@@ -139,7 +139,10 @@ grpc_cc_test(
         "//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",
+        "//test/core/tsi/test_creds/crl_data/crls:evil.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",
         "//test/core/tsi/test_creds/crl_data/crls_missing_intermediate:ab06acdd.r0",
         "//test/core/tsi/test_creds/crl_data/crls_missing_root:b9322cac.r0",
     ],

test/core/tsi/crl_ssl_transport_security_test.cc

@@ -68,6 +68,11 @@ const char* kRevokedIntermediateCertPath =
 const char* kRootCrlPath = "test/core/tsi/test_creds/crl_data/crls/current.crl";
 const char* kIntermediateCrlPath =
     "test/core/tsi/test_creds/crl_data/crls/intermediate.crl";
+const char* kModifiedSignaturePath =
+    "test/core/tsi/test_creds/crl_data/crls/invalid_signature.crl";
+const char* kModifiedContentPath =
+    "test/core/tsi/test_creds/crl_data/crls/invalid_content.crl";
+const char* kEvilCrlPath = "test/core/tsi/test_creds/crl_data/crls/evil.crl";
 
 class CrlSslTransportSecurityTest
     : public testing::TestWithParam<tsi_tls_version> {
@@ -418,6 +423,51 @@ std::string TestNameSuffix(
   return "TLS_1_3";
 }
 
+TEST_P(CrlSslTransportSecurityTest, CrlProviderModifiedContentCrl) {
+  std::string root_crl =
+      grpc_core::testing::GetFileContents(kModifiedContentPath);
+  std::string intermediate_crl =
+      grpc_core::testing::GetFileContents(kIntermediateCrlPath);
+
+  absl::StatusOr<std::shared_ptr<grpc_core::experimental::CrlProvider>>
+      provider = grpc_core::experimental::CreateStaticCrlProvider(
+          {root_crl, intermediate_crl});
+  ASSERT_NE(provider.status(), absl::OkStatus()) << provider.status();
+}
+
+TEST_P(CrlSslTransportSecurityTest, CrlProviderModifiedSignatureCrl) {
+  std::string root_crl =
+      grpc_core::testing::GetFileContents(kModifiedSignaturePath);
+  std::string intermediate_crl =
+      grpc_core::testing::GetFileContents(kIntermediateCrlPath);
+
+  absl::StatusOr<std::shared_ptr<grpc_core::experimental::CrlProvider>>
+      provider = grpc_core::experimental::CreateStaticCrlProvider(
+          {root_crl, intermediate_crl});
+  ASSERT_TRUE(provider.ok()) << provider.status();
+
+  auto* fixture = new SslTsiTestFixture(kValidKeyPath, kValidCertPath,
+                                        kValidKeyPath, kValidCertPath, nullptr,
+                                        *provider, false, false, false);
+  fixture->Run();
+}
+
+TEST_P(CrlSslTransportSecurityTest, CrlFromBadCa) {
+  std::string root_crl = grpc_core::testing::GetFileContents(kEvilCrlPath);
+  std::string intermediate_crl =
+      grpc_core::testing::GetFileContents(kIntermediateCrlPath);
+
+  absl::StatusOr<std::shared_ptr<grpc_core::experimental::CrlProvider>>
+      provider = grpc_core::experimental::CreateStaticCrlProvider(
+          {root_crl, intermediate_crl});
+  ASSERT_TRUE(provider.ok()) << provider.status();
+
+  auto* fixture = new SslTsiTestFixture(kValidKeyPath, kValidCertPath,
+                                        kValidKeyPath, kValidCertPath, nullptr,
+                                        *provider, false, false, false);
+  fixture->Run();
+}
+
 // TODO(gtcooke94) Add nullptr issuer test cases - this is not simple to test
 // the way the code is currently designed - we plan to refactor ways the OpenSSL
 // callback functions are written to have more easily testable chunks in

test/core/tsi/test_creds/crl_data/BUILD

@@ -26,6 +26,7 @@ exports_files([
     "intermediate_ca.key",
     "intermediate_ca.pem",
     "evil_ca.pem",
+    "evil_ca.key",
     "ca_with_akid.pem",
     "crl_with_akid.crl",
 ])

test/core/tsi/test_creds/crl_data/README

@@ -55,6 +55,12 @@ Generate a CA and CRL with an Authority Key Identifier
 ----------------------------------------------------------------------------
 Run `ca_with_akid_gen.sh` from the `test/core/tsi/test_creds/crl_data` directory
 
+Create CRLs with modified signatures and content
+----------------------------------------------------------------------------
+Make two copies of `test/core/tsi/test_creds/crl_data/crls/current.crl` named `test/core/tsi/test_creds/crl_data/crls/invalid_content.crl` and `test/core/tsi/test_creds/crl_data/crls/invalid_signature.crl`.
+In `invalid_content.crl`, change the first letter on the second line.
+In `invalid_signature.crl`, change the last letter before the `=` on the second to last line.
+
 
 Clean up:
 ---------

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

@@ -21,4 +21,5 @@ exports_files([
     "intermediate.crl",
     "invalid_signature.crl",
     "invalid_content.crl",
+    "evil.crl",
 ])

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

@@ -0,0 +1,12 @@
+-----BEGIN X509 CRL-----
+MIIB0TCBugIBATANBgkqhkiG9w0BAQsFADBWMQswCQYDVQQGEwJBVTETMBEGA1UE
+CAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRk
+MQ8wDQYDVQQDDAZ0ZXN0Y2EXDTI0MDMwNjIxMjE0M1oXDTM0MDMwNDIxMjE0M1qg
+MDAuMB8GA1UdIwQYMBaAFNRNe7qb2nAx+OXMM6aMHKZtclpDMAsGA1UdFAQEAgIQ
+ADANBgkqhkiG9w0BAQsFAAOCAQEAfynY04pFrcIOUmlKAqchQXlRfdfRHLKmXmRL
+L16p//b+Aq6jns8WOJ6DmfCBdy8h+kQwyh1HEB1yxYQGn3OJwR0NRK8riBhyhxkx
+akyP1TNMLYPsK/JUBqAvgIfk37oFLKhDO8etYDBndNcNdFs6hryKe40A6eULJXGE
+TXY8dTtT++fRX6VbeAaT02d0F+OHhuBEk/WncuGCe1StFEiLau8ZEalB02vv05Wy
+H8pn+O4P1oEMg0g/jeMWCqnrJQE3Ut7t2LSLBTgHGTk0cOXyYP2LcO0SVeAbjhtq
+qzUSWoxJu98N3y+hqu3FMJA/k0Z0d6PeZ50D3FjbUkT0ZM9f/g==
+-----END X509 CRL-----

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

@@ -0,0 +1,43 @@
+[req]
+distinguished_name = req_distinguished_name
+req_extensions = v3_req
+
+[req_distinguished_name]
+countryName           = Country Name (2 letter code)
+countryName_default = AU
+stateOrProvinceName   = State or Province Name (full name)
+stateOrProvinceName_default = Some-State
+organizationName          = Organization Name (eg, company)
+organizationName_default = Internet Widgits Pty Ltd
+commonName            = Common Name (eg, YOUR name)
+commonName_default = testca
+
+[crl_ext]
+authorityKeyIdentifier=keyid:always
+
+[v3_req]
+keyUsage = critical, digitalSignature, keyEncipherment, keyCertSign, cRLSign
+basicConstraints = critical, CA:true
+
+[ca]
+default_ca = CA_evil
+
+[CA_evil]
+dir               = .
+certs             = $dir/certs
+crl_dir           = $dir/crl
+new_certs_dir     = $dir/newcerts
+database          = $dir/index.txt
+serial            = $dir/serial
+RANDFILE          = $dir/private/.rand
+private_key = $dir/evil_ca.key
+certificate = $dir/evil_ca.pem
+crl = $dir/evil.crl
+
+# For certificate revocation lists.
+crlnumber         = $dir/crlnumber
+crl               = $dir/crl/evil.crl
+crl_extensions    = crl_ext
+default_crl_days  = 3650
+
+default_md = sha256

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

@@ -1,28 +1,28 @@
 -----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=
+MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDahYJBV8f013Vm
+W+1SiJ/pQFyQCvsCCsy3fUHlUVImzLk7QqSknniehz4XbS12gol9cq4uu18sINFk
+bvNNxPiuUDpLn6uguisKwM15h5x8SR0YoYepebs2yFahlL7Qo+vxxTfqamhyKmaE
+1d+EvU/l4oAcWp/BQbkInPK2o4S9SJJ7hDlJOWDbYxkn+G2sDwyeWjVGuDbyLl1n
+ngp43pd2JcCFnexculaQ9k8yXh6i5wv5SeWfMKgszUzV6SIToDUud4OqXHiz2wmy
+kEL+nHb7yjQBjqFJNdsBaJOE3Rq0/VGRGq9R3rrv6CJehU5pRxG/QAd5x3G2A20w
+WBkeTxarAgMBAAECggEAGMCZvgwS7sV/G16bVcd7EaFEOt67iwItKTWrgq3A9/sl
+mjRU0P7QW+im3GF2Dl//8fFNEKcRwz5eaZl1vt/qaVhWGh3Wg4jC+l9XhwYY8C0Z
++iHF66kJz01HHttp99kxjzvPNyLhfNkXrsFJJdCJ3djXuR58zRfEPVkF1zFThlsd
+9HIEU7vgl0zrtnq8cm1+jTpCd6Cv74INtFsEYqAPWdmR+32z3OqSOSJtY8rMi5Ly
+ZxjdHaGNT3k/8eA1yDfYmIpYgWQimzEH3FUZlwVFlfEk9GOsbTK0XDEwjTJn68LO
+5FNIpfRB2+HpdS4n6w07SLe4lJ5Mv9cJmAs1k7n/2QKBgQD30OhISkA8XlzgtnIB
+aN5A3Fv41SRVAO8PKdWKxJJcpeii72OdoHtIHY8FvGdyuRNhinkYhuVydwwRhIrz
+XLvVlpVpywEhesDoLoij0hgO+lNfxbEiH18S6Q5LuC5932UwLgbBezHklYYeNpRV
+WYWtRYeucNFPLi5DA4H+/y/faQKBgQDhvPBeZCmidMyYb6W85nHr9Y4IPLTqx/On
+wCEQwsiMnXq6nM7bacKhJKs4wy+3KO9ObNc+Fd/BnXU/JC/l5tJpwMjW00g8byKu
+cZlOglaOADtlxueXCBLV27V8L7Cx5iOicV2ouOaJjMps4k/fWIOIK1XZHj4txJXj
+C/7/RvMW8wKBgDU8tNnq0Zfmca94ok85Nx3Z+QwgxdhZBgJM62oPRp4OqkZuhQj0
+0+cvKm2CBvs3VTmMJO2m9R29A2O0BKG5V0TQP7LlgI2vsEdwz7vZw39cOZMGhkId
+WTBXztFndN1no3ZRPPRNwe8oTBKriPw46iXKHRbVd7G56whMdZ3RNniZAoGBAL1x
+qugMd0R3cRyc1iLp4sF7mm8fQ1Wl6L3nZ9iBH32iy9TAtHk/EK/b7jX82JaGLA9N
+GHZqNRZv5m3PGMOAKyXFPMfNGNpfCmQLwfU5PRp+51pKyyDdDbGcaXqHK2qhEVbK
+fSeTxSW6mkc2xoFR71DfzXQhBV2zlXauIppqGKgxAoGAMh6fd4tFBa2lpGsfK1Kp
+bXKmSMe0P9aV+JrjWHACXcS0n+v/LvNLZrddp+RF4t2M5U7ZujHJEJAHafCo56qk
+MkV2WseGb7CBlFQjLKYVXJsiNCf5O5ME+cckT2bK1aX0/h8dOtXirwuQ4OVw2c+n
+7/MDH+S5WiEX51Cu4zdVMis=
 -----END PRIVATE KEY-----

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

@@ -1,21 +1,21 @@
 -----BEGIN CERTIFICATE-----
-MIIDeTCCAmGgAwIBAgIUULA9nt1NB3W1i4RevrKeRQQLkaIwDQYJKoZIhvcNAQEL
+MIIDeTCCAmGgAwIBAgIURqastxiKmyjvJwoaXfh8hA1mccIwDQYJKoZIhvcNAQEL
 BQAwVjELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
 GEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDEPMA0GA1UEAwwGdGVzdGNhMB4XDTI0
-MDEyMjIxNDAyMFoXDTM0MDExOTIxNDAyMFowVjELMAkGA1UEBhMCQVUxEzARBgNV
+MDMwNjE5NDcwMVoXDTM0MDMwNDE5NDcwMVowVjELMAkGA1UEBhMCQVUxEzARBgNV
 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=
+AQEAlDQ88qcz8a9SdLslQrRsN6EJkgWS1dQZ9mzgeGdNyWULlqmOjqP7JZecQSfG
+KKA01wkmnzQwaw2HY+kcDw48HKBkjOVVctat4sFg4Du7cwxZPhDTnqxLs1U5poNH
+w53gwYd62NDYmGk10J5MbgMmREqPnAVWKHSpNdGErJ9T/AJxlc/QyMKICmt6Iond
+TUOWFti3e/K9fqTi9d9Oa6u7hxRky2ZWn3t1NE/p1UMDFcG3Ugn9YkGB6ZPbzno2
+vNWwN3UmV2HOW2QzVmghUm8KlkvaNdRJ5+YvdEAktNS6NNVkoqXo2cfFdQkTtHu/
+OdFCmsIyGBkrpTi4Rq6ObBE+/QIDAQABoz8wPTAMBgNVHRMEBTADAQH/MA4GA1Ud
+DwEB/wQEAwIBBjAdBgNVHQ4EFgQUBDQP4CFbiBzHqvAh6TVpA78MeJYwDQYJKoZI
+hvcNAQELBQADggEBABwNHIQXyV+8mvvKpC47rUtRvMuFruRmqZb2lET/NiVzazq/
+s3FNNFKTc8DOQzWYhxF5kMSd0+pL7zK7qAkTi+/Gxc7bJpyFvxQZ6FvVgtz2skv1
+8MD3FIcfq3VhbHQnmbp8AY1YGM2uvduSReLEWTz7SIx8bZxDxl8g6K5V71XIWM5X
+CHuk7GybN5gemI+WE1a+1wXcL6FVaWCQHrJVT2ZNS1r5rVXOObf8Ubh5gR2kaVY8
+f69OkJ0+XDCOXQw3zmTafnKBtXYdYdT/lMIh8OiseX08W33EiBDczJqFeS/crJZ+
+Tlj+UK1Mtt1NyFu1YV/C7dmGSAZUsdTVM0nY5HA=
 -----END CERTIFICATE-----

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

@@ -14,5 +14,21 @@
 # limitations under the License.
 
 # Generates a CA with the same issuer name as the good CA in this directory
+rm -rf evil_ca
+mkdir evil_ca
+cp evil_ca.cnf evil_ca/
+pushd evil_ca  || exit
+touch index.txt
+echo 1 > ./serial
+echo 1000 > ./crlnumber
+
+# Generate the CA with the same subject as the good CA
 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
+  -config evil_ca.cnf -days 3650 -extensions v3_req
+
+# Generate the CRL file:
+# ----------------------------------------------------------------------------
+openssl ca -config=evil_ca.cnf -gencrl -out evil.crl -keyfile evil_ca.key -cert evil_ca.pem -crldays 3650
+popd || exit
+cp "./evil_ca/evil.crl" ./crls/
+rm -rf evil_ca