From 2d34ccff423a11a7afa9edb0ab323c7eec1a16fb Mon Sep 17 00:00:00 2001
From: krestofur <83723727+krestofur@users.noreply.github.com>
Date: Thu, 24 Mar 2022 15:00:25 -0700
Subject: [PATCH] Softfail when receiving a X509_V_ERR_UNABLE_TO_GET_CRL error
 (#29124)

---
 src/core/tsi/ssl_transport_security.cc        |  8 ++++
 .../tsi/crl_ssl_transport_security_test.cc    | 38 +++++++++++++++----
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/src/core/tsi/ssl_transport_security.cc b/src/core/tsi/ssl_transport_security.cc
index 2208681e4ea..44352ce977b 100644
--- a/src/core/tsi/ssl_transport_security.cc
+++ b/src/core/tsi/ssl_transport_security.cc
@@ -1939,8 +1939,16 @@ static void ssl_keylogging_callback(const SSL* ssl, const char* info) {
   factory->key_logger->LogSessionKeys(ssl_context, info);
 }
 
+// This callback is invoked when the CRL has been verified and will soft-fail
+// errors in verification depending on certain error types.
 static int verify_cb(int ok, X509_STORE_CTX* ctx) {
   int cert_error = X509_STORE_CTX_get_error(ctx);
+  if (cert_error == X509_V_ERR_UNABLE_TO_GET_CRL) {
+    gpr_log(
+        GPR_INFO,
+        "Certificate verification failed to get CRL files. Ignoring error.");
+    return 1;
+  }
   if (cert_error != 0) {
     gpr_log(GPR_ERROR, "Certificate verify failed with code %d", cert_error);
   }
diff --git a/test/core/tsi/crl_ssl_transport_security_test.cc b/test/core/tsi/crl_ssl_transport_security_test.cc
index 17a04585f8e..eede108d3db 100644
--- a/test/core/tsi/crl_ssl_transport_security_test.cc
+++ b/test/core/tsi/crl_ssl_transport_security_test.cc
@@ -43,6 +43,7 @@ const int kSslTsiTestRevokedKeyCertPairsNum = 1;
 const int kSslTsiTestValidKeyCertPairsNum = 1;
 const char* kSslTsiTestCrlSupportedCredentialsDir =
     "test/core/tsi/test_creds/crl_data/";
+const char* kSslTsiTestFaultyCrlsDir = "bad_path/";
 
 class CrlSslTransportSecurityTest
     : public testing::TestWithParam<tsi_tls_version> {
@@ -50,10 +51,14 @@ class CrlSslTransportSecurityTest
   // A tsi_test_fixture implementation.
   class SslTsiTestFixture {
    public:
+    // When use_faulty_crl_directory is set, the crl_directory of the
+    // client is set to a non-existant path.
     static SslTsiTestFixture* Create(bool use_revoked_server_cert,
-                                     bool use_revoked_client_cert) {
+                                     bool use_revoked_client_cert,
+                                     bool use_faulty_crl_directory) {
       return new SslTsiTestFixture(use_revoked_server_cert,
-                                   use_revoked_client_cert);
+                                   use_revoked_client_cert,
+                                   use_faulty_crl_directory);
     }
 
     void Run() {
@@ -63,9 +68,11 @@ class CrlSslTransportSecurityTest
 
    private:
     SslTsiTestFixture(bool use_revoked_server_cert,
-                      bool use_revoked_client_cert)
+                      bool use_revoked_client_cert,
+                      bool use_faulty_crl_directory)
         : use_revoked_server_cert_(use_revoked_server_cert),
-          use_revoked_client_cert_(use_revoked_client_cert) {
+          use_revoked_client_cert_(use_revoked_client_cert),
+          use_faulty_crl_directory_(use_faulty_crl_directory) {
       tsi_test_fixture_init(&base_);
       base_.test_unused_bytes = true;
       base_.vtable = &kVtable;
@@ -120,7 +127,11 @@ class CrlSslTransportSecurityTest
       } else {
         client_options.pem_key_cert_pair = valid_pem_key_cert_pairs_;
       }
-      client_options.crl_directory = kSslTsiTestCrlSupportedCredentialsDir;
+      if (use_faulty_crl_directory_) {
+        client_options.crl_directory = kSslTsiTestFaultyCrlsDir;
+      } else {
+        client_options.crl_directory = kSslTsiTestCrlSupportedCredentialsDir;
+      }
       client_options.root_store = root_store_;
       client_options.min_tls_version = GetParam();
       client_options.max_tls_version = GetParam();
@@ -228,6 +239,7 @@ class CrlSslTransportSecurityTest
     tsi_test_fixture base_;
     bool use_revoked_server_cert_;
     bool use_revoked_client_cert_;
+    bool use_faulty_crl_directory_;
     char* root_cert_;
     tsi_ssl_root_certs_store* root_store_;
     tsi_ssl_pem_key_cert_pair* revoked_pem_key_cert_pairs_;
@@ -245,19 +257,29 @@ struct tsi_test_fixture_vtable
 
 TEST_P(CrlSslTransportSecurityTest, RevokedServerCert) {
   auto* fixture = SslTsiTestFixture::Create(/*use_revoked_server_cert=*/true,
-                                            /*use_revoked_client_cert=*/false);
+                                            /*use_revoked_client_cert=*/false,
+                                            /*use_faulty_crl_directory=*/false);
   fixture->Run();
 }
 
 TEST_P(CrlSslTransportSecurityTest, RevokedClientCert) {
   auto* fixture = SslTsiTestFixture::Create(/*use_revoked_server_cert=*/false,
-                                            /*use_revoked_client_cert=*/true);
+                                            /*use_revoked_client_cert=*/true,
+                                            /*use_faulty_crl_directory=*/false);
   fixture->Run();
 }
 
 TEST_P(CrlSslTransportSecurityTest, ValidCerts) {
   auto* fixture = SslTsiTestFixture::Create(/*use_revoked_server_cert=*/false,
-                                            /*use_revoked_client_cert=*/false);
+                                            /*use_revoked_client_cert=*/false,
+                                            /*use_faulty_crl_directory=*/false);
+  fixture->Run();
+}
+
+TEST_P(CrlSslTransportSecurityTest, UseFaultyCrlDirectory) {
+  auto* fixture = SslTsiTestFixture::Create(/*use_revoked_server_cert=*/false,
+                                            /*use_revoked_client_cert=*/false,
+                                            /*use_faulty_crl_directory=*/true);
   fixture->Run();
 }