Add support for CRLSet to CertVerifyProcBuiltin.

Bug: 649017
Change-Id: I7196aeafa9674dfd5e25cb828d4acb464f4c0a2e
Reviewed-on: https://chromium-review.googlesource.com/686043Reviewed-by: Matt Mueller <mattm@chromium.org>
Commit-Queue: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504856}

net/cert/cert_verify_proc_builtin.cc

@@ -17,6 +17,7 @@
 #include "net/cert/cert_status_flags.h"
 #include "net/cert/cert_verify_proc.h"
 #include "net/cert/cert_verify_result.h"
+#include "net/cert/crl_set.h"
 #include "net/cert/internal/cert_errors.h"
 #include "net/cert/internal/cert_issuer_source_static.h"
 #include "net/cert/internal/common_cert_errors.h"
@@ -32,6 +33,117 @@ namespace net {
 
 namespace {
 
+DEFINE_CERT_ERROR_ID(kCertificateRevoked, "Certificate is revoked");
+
+// Enum to indicate whether the revocation status for a certificate is
+// known. When the status is "known" it means it was either revoked, or
+// affirmatively unrevoked.
+enum class CertRevocationStatus {
+  kUnknown,
+  kKnown,
+};
+
+// TODO(eroman): The path building code in this file enforces its idea of weak
+// keys, and separately cert_verify_proc.cc also checks the chains with its
+// own policy. These policies should be aligned, to give path building the
+// best chance of finding a good path.
+class PathBuilderDelegateImpl : public SimplePathBuilderDelegate {
+ public:
+  // Uses the default policy from SimplePathBuilderDelegate, which requires RSA
+  // keys to be at least 1024-bits large, and accepts SHA1 certificates.
+  PathBuilderDelegateImpl(CRLSet* crl_set)
+      : SimplePathBuilderDelegate(1024), crl_set_(crl_set) {}
+
+  // This is called for each built chain, including ones which failed. It is
+  // responsible for adding errors to the built chain if it is not acceptable.
+  void CheckPathAfterVerification(const CertPath& path,
+                                  CertPathErrors* errors) override {
+    CheckRevocation(path, errors);
+  }
+
+ private:
+  // This method checks whether a certificate chain has been revoked, and if
+  // so adds errors to the affected certificates.
+  CertRevocationStatus CheckRevocation(const CertPath& path,
+                                       CertPathErrors* errors) const {
+    // First check for revocations using the CRLSet. This does not require
+    // any network activity.
+    if (crl_set_) {
+      CertRevocationStatus status = CheckRevocationUsingCRLSet(path, errors);
+      if (status == CertRevocationStatus::kKnown)
+        return status;
+    }
+
+    // TODO(eroman): Next check revocation using OCSP and CRL.
+    return CertRevocationStatus::kUnknown;
+  }
+
+  // Checks the revocation status of the certificate chain using the CRLSet. If
+  // any certificate is revoked, the kCertificateRevoked high-severity error is
+  // added.
+  CertRevocationStatus CheckRevocationUsingCRLSet(
+      const CertPath& path,
+      CertPathErrors* errors) const {
+    // Iterate from root certificate towards the leaf (the root certificate is
+    // also checked for revocation by CRLSet).
+    std::string issuer_spki_hash;
+    for (size_t reverse_i = 0; reverse_i < path.certs.size(); ++reverse_i) {
+      size_t i = path.certs.size() - reverse_i - 1;
+      const scoped_refptr<ParsedCertificate>& cert = path.certs[i];
+
+      // True if |cert| is the root of the chain.
+      const bool is_root = reverse_i == 0;
+      // True if |cert| is the leaf certificate of the chain.
+      const bool is_target = i == 0;
+
+      // Check for revocation using the certificate's SPKI.
+      std::string spki_hash =
+          crypto::SHA256HashString(cert->tbs().spki_tlv.AsStringPiece());
+      CRLSet::Result result = crl_set_->CheckSPKI(spki_hash);
+
+      // Check for revocation using the certificate's serial number and issuer's
+      // SPKI.
+      if (result != CRLSet::REVOKED && !is_root) {
+        result = crl_set_->CheckSerial(
+            cert->tbs().serial_number.AsStringPiece(), issuer_spki_hash);
+      }
+
+      // Prepare for the next iteration.
+      issuer_spki_hash = std::move(spki_hash);
+
+      switch (result) {
+        case CRLSet::REVOKED:
+          // If any certificate was revoked, add an error to the chain and
+          // return (no need to check the remaining certificates).
+          errors->GetErrorsForCert(i)->AddError(kCertificateRevoked);
+          return CertRevocationStatus::kKnown;
+        case CRLSet::UNKNOWN:
+          // If the status is unknown, advance to the subordinate certificate.
+          break;
+        case CRLSet::GOOD:
+          if (is_target && !crl_set_->IsExpired()) {
+            // If the target is covered by the CRLSet and known good, consider
+            // the entire chain to be valid (even though the revocation status
+            // of the intermediates may have been UNKNOWN).
+            //
+            // 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.
+            return CertRevocationStatus::kKnown;
+          }
+          break;
+      }
+    }
+
+    // If no certificate was revoked, and the target was not known good, then
+    // the revocation status is still unknown.
+    return CertRevocationStatus::kUnknown;
+  }
+
+  // The CRLSet may be null.
+  CRLSet* crl_set_;
+};
+
 class CertVerifyProcBuiltin : public CertVerifyProc {
  public:
   CertVerifyProcBuiltin();
@@ -115,6 +227,9 @@ void MapPathBuilderErrorsToCertStatus(const CertPathErrors& errors,
   if (!errors.ContainsHighSeverityErrors())
     return;
 
+  if (errors.ContainsError(kCertificateRevoked))
+    *cert_status |= CERT_STATUS_REVOKED;
+
   if (errors.ContainsError(cert_errors::kUnacceptablePublicKey))
     *cert_status |= CERT_STATUS_WEAK_KEY;
 
@@ -197,11 +312,7 @@ void DoVerify(X509Certificate* input_cert,
     // TODO(eroman): Surface parsing errors of additional trust anchor.
   }
 
-  // TODO(eroman): The path building code in this file enforces its idea of weak
-  // keys, and separately cert_verify_proc.cc also checks the chains with its
-  // own policy. These policies should be aligned, to give path building the
-  // best chance of finding a good path.
-  SimplePathBuilderDelegate path_builder_delegate(1024);
+  PathBuilderDelegateImpl path_builder_delegate(crl_set);
 
   // Use the current time.
   der::GeneralizedTime verification_time;

net/cert/cert_verify_proc_unittest.cc

@@ -273,16 +273,16 @@ class CertVerifyProcInternalTest
   }
 
   bool SupportsCRLSet() const {
-    // TODO(crbug.com/649017): Return true for CERT_VERIFY_PROC_BUILTIN.
     return verify_proc_type() == CERT_VERIFY_PROC_NSS ||
            verify_proc_type() == CERT_VERIFY_PROC_WIN ||
-           verify_proc_type() == CERT_VERIFY_PROC_MAC;
+           verify_proc_type() == CERT_VERIFY_PROC_MAC ||
+           verify_proc_type() == CERT_VERIFY_PROC_BUILTIN;
   }
 
   bool SupportsCRLSetsInPathBuilding() const {
-    // TODO(crbug.com/649017): Return true for CERT_VERIFY_PROC_BUILTIN.
     return verify_proc_type() == CERT_VERIFY_PROC_WIN ||
-           verify_proc_type() == CERT_VERIFY_PROC_NSS;
+           verify_proc_type() == CERT_VERIFY_PROC_NSS ||
+           verify_proc_type() == CERT_VERIFY_PROC_BUILTIN;
   }
 
   bool SupportsEV() const {