[Security Tests - Crl Provider] Fix inconsistent directory related tests (#36122)

There were some failures in the Crl Directory related tests after https://github.com/grpc/grpc/pull/36031

This came down to https://github.com/grpc/grpc/pull/36031 adding some CRLs with bad qualities (invalid content/signatures, overriding issuer names) to the `test_creds/crl_data/crls` directory, which is used in the directory reloading tests. The tests began failing on some platforms because they were picking up these bad crls which were failing various checks, but the test was designed to assume that `test_creds/crl_data/crls` was a valid and good directory.
This PR moves the bad CRLs to their own directory to prevent this accidental mash-up of test data. It also adds debug logging to our custom verification stack.

Closes #36122

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36122 from gtcooke94:crl_fix 508dd1370d
PiperOrigin-RevId: 616280898

src/core/tsi/ssl_transport_security.cc

@@ -1045,10 +1045,12 @@ static bool ValidateCrl(X509* cert, X509* issuer, X509_CRL* crl) {
   // 6.3.3b verify issuer and scope
   valid = grpc_core::VerifyCrlCertIssuerNamesMatch(crl, cert);
   if (!valid) {
+    gpr_log(GPR_DEBUG, "CRL and cert issuer names mismatched.");
     return valid;
   }
   valid = grpc_core::HasCrlSignBit(issuer);
   if (!valid) {
+    gpr_log(GPR_DEBUG, "CRL issuer not allowed to sign CRLs.");
     return valid;
   }
   // 6.3.3c Not supporting deltas
@@ -1058,6 +1060,9 @@ static bool ValidateCrl(X509* cert, X509* issuer, X509_CRL* crl) {
   // same.
   // 6.3.3g Verify CRL Signature
   valid = grpc_core::VerifyCrlSignature(crl, issuer);
+  if (!valid) {
+    gpr_log(GPR_DEBUG, "Crl signature check failed.");
+  }
   return valid;
 }
 
@@ -1146,6 +1151,7 @@ static int CheckChainRevocation(
 static int CustomVerificationFunction(X509_STORE_CTX* ctx, void* arg) {
   int ret = X509_verify_cert(ctx);
   if (ret <= 0) {
+    gpr_log(GPR_DEBUG, "Failed to verify cert chain.");
     // 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.
@@ -1154,10 +1160,10 @@ static int CustomVerificationFunction(X509_STORE_CTX* ctx, void* arg) {
   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;
+    if (ret <= 0) {
+      gpr_log(GPR_DEBUG, "The chain failed revocation checks.");
+      return ret;
+    }
   }
   return RootCertExtractCallback(ctx, arg);
 }

src/core/tsi/ssl_transport_security_utils.cc

@@ -259,12 +259,19 @@ bool VerifyCrlSignature(X509_CRL* crl, X509* issuer) {
   if (ikey == nullptr) {
     // Can't verify signature because we couldn't get the pubkey, fail the
     // check.
+    gpr_log(GPR_DEBUG, "Could not public key from certificate.");
     EVP_PKEY_free(ikey);
     return false;
   }
-  bool ret = X509_CRL_verify(crl, ikey) == 1;
+  int ret = X509_CRL_verify(crl, ikey);
+  if (ret < 0) {
+    gpr_log(GPR_DEBUG,
+            "There was an unexpected problem checking the CRL signature.");
+  } else if (ret == 0) {
+    gpr_log(GPR_DEBUG, "CRL failed verification.");
+  }
   EVP_PKEY_free(ikey);
-  return ret;
+  return ret == 1;
 }
 
 bool VerifyCrlCertIssuerNamesMatch(X509_CRL* crl, X509* cert) {

test/core/tsi/BUILD

@@ -71,10 +71,10 @@ grpc_cc_test(
         "//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/bad_crls:invalid_content.crl",
+        "//test/core/tsi/test_creds/crl_data/bad_crls:invalid_signature.crl",
         "//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++",
@@ -136,13 +136,13 @@ grpc_cc_test(
         "//test/core/tsi/test_creds/crl_data:revoked.pem",
         "//test/core/tsi/test_creds/crl_data:valid.key",
         "//test/core/tsi/test_creds/crl_data:valid.pem",
+        "//test/core/tsi/test_creds/crl_data/bad_crls:evil.crl",
+        "//test/core/tsi/test_creds/crl_data/bad_crls:invalid_content.crl",
+        "//test/core/tsi/test_creds/crl_data/bad_crls:invalid_signature.crl",
         "//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

@@ -69,10 +69,11 @@ 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";
+    "test/core/tsi/test_creds/crl_data/bad_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";
+    "test/core/tsi/test_creds/crl_data/bad_crls/invalid_content.crl";
+const char* kEvilCrlPath =
+    "test/core/tsi/test_creds/crl_data/bad_crls/evil.crl";
 
 class CrlSslTransportSecurityTest
     : public testing::TestWithParam<tsi_tls_version> {

test/core/tsi/ssl_transport_security_utils_test.cc

@@ -46,9 +46,9 @@ 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";
+    "test/core/tsi/test_creds/crl_data/bad_crls/invalid_signature.crl";
 const char* kModifiedContent =
-    "test/core/tsi/test_creds/crl_data/crls/invalid_content.crl";
+    "test/core/tsi/test_creds/crl_data/bad_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/README

@@ -57,7 +57,8 @@ 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`.
+Make a directory `test/core/tsi/test_creds/crl_data/bad_crls
+Make two copies of `test/core/tsi/test_creds/crl_data/crls/current.crl` in the bad_crls directory named `test/core/tsi/test_creds/crl_data/bad_crls/invalid_content.crl` and `test/core/tsi/test_creds/crl_data/bad_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.
 

test/core/tsi/test_creds/crl_data/bad_crls/BUILD

@@ -0,0 +1,21 @@
+# Copyright 2021 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.
+
+licenses(["notice"])
+
+exports_files([
+    "evil.crl",
+    "invalid_content.crl",
+    "invalid_signature.crl",
+])

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

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

test/cpp/end2end/crl_provider_test.cc

@@ -61,7 +61,8 @@ 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/";
+const char* kCrlDirectoryPath =
+    "test/core/tsi/test_creds/crl_data/crl_provider_test_dir/";
 constexpr char kMessage[] = "Hello";
 
 // This test must be at the top of the file because the