Perform CRLSet evaluation during Path Building on NSS

net/cert/cert_verify_proc_nss.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/cert/cert_verify_proc_nss.h"
 
 #include <string>
 #include <vector>
 
 #include <cert.h>
@@ skipping 261 matching lines @@
 //   kCRLSetRevoked: if any element of the chain is known to have been revoked.
 //   kCRLSetUnknown: if there is no fresh information about the leaf
 //       certificate in the chain or if the CRLSet has expired.
 //
 //       Only the leaf certificate is considered for coverage because some
 //       intermediates have CRLs with no revocations (after filtering) and
 //       those CRLs are pruned from the CRLSet at generation time. This means
 //       that some EV sites would otherwise take the hit of an OCSP lookup for
 //       no reason.
 //   kCRLSetOk: otherwise.
-CRLSetResult CheckRevocationWithCRLSet(CERTCertList* cert_list,
+CRLSetResult CheckRevocationWithCRLSet(const CERTCertList* cert_list,
                                        CERTCertificate* root,
                                        CRLSet* crl_set) {
   std::vector<CERTCertificate*> certs;
 
   if (cert_list) {
     for (CERTCertListNode* node = CERT_LIST_HEAD(cert_list);
          !CERT_LIST_END(node, cert_list);
          node = CERT_LIST_NEXT(node)) {
       certs.push_back(node->cert);
     }
@@ skipping 52 matching lines @@
         error = true;
         continue;
     }
   }
 
   if (error || !last_covered || crl_set->IsExpired())
     return kCRLSetUnknown;
   return kCRLSetOk;
 }
 
+// Arguments for CheckChainRevocationWithCRLSet that are curried within the
+// CERTChainVerifyCallback's isChainValidArg.
+struct CheckChainRevocationArgs {
+  // The CRLSet to evaluate against.
+  CRLSet* crl_set = nullptr;
+
+  // The next callback to invoke, if the CRLSet does not report any errors.
+  CERTChainVerifyCallback* next_callback = nullptr;
+
+  // Indicates that the application callback failure was due to a CRLSet
+  // revocation, rather than due to |next_callback| rejecting it. This is
+  // used to map the error back to the proper caller-visible error code.
+  bool was_revoked = false;
+};
+
+SECStatus CheckChainRevocationWithCRLSet(void* is_chain_valid_arg,
+                                         const CERTCertList* current_chain,
+                                         PRBool* chain_ok) {
+  CHECK(is_chain_valid_arg);
+
+  CheckChainRevocationArgs* args =
+      static_cast<CheckChainRevocationArgs*>(is_chain_valid_arg);
+
+  CRLSetResult crlset_result = kCRLSetUnknown;
+  if (args->crl_set) {
+    crlset_result =
+        CheckRevocationWithCRLSet(current_chain, nullptr, args->crl_set);
+  }
+
+  if (crlset_result == kCRLSetRevoked) {
+    args->was_revoked = true;
+    *chain_ok = PR_FALSE;
+    return SECSuccess;
+  }
+  args->was_revoked = false;
+
+  *chain_ok = PR_TRUE;
+  if (!args->next_callback || !args->next_callback->isChainValid)
+    return SECSuccess;
+
+  return (*args->next_callback->isChainValid)(
+      args->next_callback->isChainValidArg, current_chain, chain_ok);
+}
+
 // Forward declarations.
 SECStatus RetryPKIXVerifyCertWithWorkarounds(
     CERTCertificate* cert_handle, int num_policy_oids,
     bool cert_io_enabled, std::vector<CERTValInParam>* cvin,
     CERTValOutParam* cvout);
 SECOidTag GetFirstCertPolicy(CERTCertificate* cert_handle);
 
 // Call CERT_PKIXVerifyCert for the cert_handle.
 // Verification results are stored in an array of CERTValOutParam.
 // If |hard_fail| is true, and no policy_oids are supplied (eg: EV is NOT being
@@ skipping 453 matching lines @@
     cache_ocsp_response_from_side_channel_(CERT_GetDefaultCertDB(), cert_handle,
                                            PR_Now(), &ocsp_response_item,
                                            nullptr);
   }
 
   if (!cert->VerifyNameMatch(hostname,
                              &verify_result->common_name_fallback_used)) {
     verify_result->cert_status |= CERT_STATUS_COMMON_NAME_INVALID;
   }
 
+  CheckChainRevocationArgs check_chain_revocation_args;
+  CERTChainVerifyCallback crlset_callback;
+  memset(&crlset_callback, 0, sizeof(crlset_callback));
+  crlset_callback.isChainValid = &CheckChainRevocationWithCRLSet;
+  crlset_callback.isChainValidArg =
+      static_cast<void*>(&check_chain_revocation_args);
+
   // Make sure that the cert is valid now.
   SECCertTimeValidity validity = CERT_CheckCertValidTimes(
       cert_handle, PR_Now(), PR_TRUE);
   if (validity != secCertTimeValid)
     verify_result->cert_status |= CERT_STATUS_DATE_INVALID;
 
   CERTValOutParam cvout[3];
   int cvout_index = 0;
   cvout[cvout_index].type = cert_po_certList;
   cvout[cvout_index].value.pointer.chain = NULL;
@@ skipping 17 matching lines @@
       (flags & CertVerifier::VERIFY_REV_CHECKING_ENABLED);
   if (check_revocation)
     verify_result->cert_status |= CERT_STATUS_REV_CHECKING_ENABLED;
 
   ScopedCERTCertList trust_anchors;
   if (!additional_trust_anchors.empty()) {
     trust_anchors.reset(
         CertificateListToCERTCertList(additional_trust_anchors));
   }
 
-  SECStatus status = PKIXVerifyCert(cert_handle,
-                                    check_revocation,
-                                    false,
-                                    cert_io_enabled,
-                                    NULL,
-                                    0,
-                                    trust_anchors.get(),
-                                    chain_verify_callback,
-                                    cvout);
+  SECStatus status =
+      PKIXVerifyCert(cert_handle, check_revocation, false, cert_io_enabled,
+                     NULL, 0, trust_anchors.get(), &crlset_callback, cvout);
 
   if (status == SECSuccess &&
       (flags & CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS) &&
       !IsKnownRoot(cvout[cvout_trust_anchor_index].value.pointer.cert)) {
     // TODO(rsleevi): Optimize this by supplying the constructed chain to
     // libpkix via cvin. Omitting for now, due to lack of coverage in upstream
     // NSS tests for that feature.
     scoped_cvout.Clear();
     verify_result->cert_status |= CERT_STATUS_REV_CHECKING_ENABLED;
-    status = PKIXVerifyCert(cert_handle,
-                            true,
-                            true,
-                            cert_io_enabled,
-                            NULL,
-                            0,
-                            trust_anchors.get(),
-                            chain_verify_callback,
-                            cvout);
+    status = PKIXVerifyCert(cert_handle, true, true, cert_io_enabled, NULL, 0,
+                            trust_anchors.get(), &crlset_callback, cvout);

[eroman, 2016/03/02 00:05:07]

I think it would be better to explicitly reset |check_chain_revocation_args|
before each call to PKIXVerifyCert(), rather than relying on any mutations from
the previous call to not affect the final value of |check_chain_revocation_args|
which is tested for later.

Maybe just check_chain_revocation_args.Reset() before each PKIXVerifyCert() call
that links to it.

I can't tell if the current code not re-setting was_revoked between calls leads
to problems. At the very least it is subtle.

   }
 
   if (status == SECSuccess) {
     AppendPublicKeyHashes(cvout[cvout_cert_list_index].value.pointer.chain,
                           cvout[cvout_trust_anchor_index].value.pointer.cert,
                           &verify_result->public_key_hashes);
 
     verify_result->is_issued_by_known_root =
         IsKnownRoot(cvout[cvout_trust_anchor_index].value.pointer.cert);
     verify_result->is_issued_by_additional_trust_anchor =
         IsAdditionalTrustAnchor(
             trust_anchors.get(),
             cvout[cvout_trust_anchor_index].value.pointer.cert);
 
     GetCertChainInfo(cvout[cvout_cert_list_index].value.pointer.chain,
                      cvout[cvout_trust_anchor_index].value.pointer.cert,
                      verify_result);
   }
 
   CRLSetResult crl_set_result = kCRLSetUnknown;
   if (crl_set) {
-    crl_set_result = CheckRevocationWithCRLSet(
-        cvout[cvout_cert_list_index].value.pointer.chain,
-        cvout[cvout_trust_anchor_index].value.pointer.cert,
-        crl_set);
-    if (crl_set_result == kCRLSetRevoked) {
+    if (status == SECSuccess) {
+      crl_set_result = CheckRevocationWithCRLSet(
+          cvout[cvout_cert_list_index].value.pointer.chain,
+          cvout[cvout_trust_anchor_index].value.pointer.cert, crl_set);
+      if (crl_set_result == kCRLSetRevoked) {
+        PORT_SetError(SEC_ERROR_REVOKED_CERTIFICATE);
+        status = SECFailure;
+      }
+    } else if (PORT_GetError() == SEC_ERROR_APPLICATION_CALLBACK_ERROR &&
+               check_chain_revocation_args.was_revoked) {
+      // If a CRLSet was supplied, and the error was an application callback
+      // error, then it was directed through the CRLSet code and that
+      // particular chain was revoked.
       PORT_SetError(SEC_ERROR_REVOKED_CERTIFICATE);
-      status = SECFailure;
     }
   }
 
   if (status != SECSuccess) {
     int err = PORT_GetError();
     LOG(ERROR) << "CERT_PKIXVerifyCert for " << hostname
                << " failed err=" << err;
     // CERT_PKIXVerifyCert rerports the wrong error code for
     // expired certificates (NSS bug 491174)
     if (err == SEC_ERROR_CERT_NOT_VALID &&
@@ skipping 12 matching lines @@
     return MapCertStatusToNetError(verify_result->cert_status);
 
   if ((flags & CertVerifier::VERIFY_EV_CERT) && is_ev_candidate) {
     check_revocation |=
         crl_set_result != kCRLSetOk &&
         cert_io_enabled &&
         (flags & CertVerifier::VERIFY_REV_CHECKING_ENABLED_EV_ONLY);
     if (check_revocation)
       verify_result->cert_status |= CERT_STATUS_REV_CHECKING_ENABLED;
 
-    if (VerifyEV(cert_handle,
-                 flags,
-                 crl_set,
-                 check_revocation,
-                 metadata,
-                 ev_policy_oid,
-                 trust_anchors.get(),
-                 chain_verify_callback)) {
+    if (VerifyEV(cert_handle, flags, crl_set, check_revocation, metadata,
+                 ev_policy_oid, trust_anchors.get(), &crlset_callback)) {
       verify_result->cert_status |= CERT_STATUS_IS_EV;
     }
   }
 
   return OK;
 }
 
 int CertVerifyProcNSS::VerifyInternal(
     X509Certificate* cert,
     const std::string& hostname,
     const std::string& ocsp_response,
     int flags,
     CRLSet* crl_set,
     const CertificateList& additional_trust_anchors,
     CertVerifyResult* verify_result) {
   return VerifyInternalImpl(cert, hostname, ocsp_response, flags, crl_set,
                             additional_trust_anchors,
                             NULL,  // chain_verify_callback
                             verify_result);
 }
 
 }  // namespace net