Bug 1045739 - Stop checking revocation for expired certificates. r=keeler, a=lmandel
authorBrian Smith <brian@briansmith.org>
Thu, 14 Aug 2014 12:02:55 -0700
changeset 225487 db731c467d75686eff16f58534fde054b2f24e26
parent 225486 b0dce6c46c52ef9c5110d8aa080fc6ded53d3da5
child 225488 3e7f5082d9ec8451c3b2084c904061d4723180b8
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler, lmandel
bugs1045739
milestone34.0a2
Bug 1045739 - Stop checking revocation for expired certificates. r=keeler, a=lmandel
security/pkix/lib/pkixbuild.cpp
--- a/security/pkix/lib/pkixbuild.cpp
+++ b/security/pkix/lib/pkixbuild.cpp
@@ -45,24 +45,25 @@ TrustDomain::IssuerChecker::~IssuerCheck
 // hide the implementation from external users.
 class PathBuildingStep : public TrustDomain::IssuerChecker
 {
 public:
   PathBuildingStep(TrustDomain& trustDomain, const BackCert& subject,
                    Time time, KeyPurposeId requiredEKUIfPresent,
                    const CertPolicyId& requiredPolicy,
                    /*optional*/ const Input* stapledOCSPResponse,
-                   unsigned int subCACount)
+                   unsigned int subCACount, Result deferredSubjectError)
     : trustDomain(trustDomain)
     , subject(subject)
     , time(time)
     , requiredEKUIfPresent(requiredEKUIfPresent)
     , requiredPolicy(requiredPolicy)
     , stapledOCSPResponse(stapledOCSPResponse)
     , subCACount(subCACount)
+    , deferredSubjectError(deferredSubjectError)
     , result(Result::FATAL_ERROR_LIBRARY_FAILURE)
     , resultWasSet(false)
   {
   }
 
   Result Check(Input potentialIssuerDER,
                /*optional*/ const Input* additionalNameConstraints,
                /*out*/ bool& keepGoing);
@@ -72,16 +73,17 @@ public:
 private:
   TrustDomain& trustDomain;
   const BackCert& subject;
   const Time time;
   const KeyPurposeId requiredEKUIfPresent;
   const CertPolicyId& requiredPolicy;
   /*optional*/ Input const* const stapledOCSPResponse;
   const unsigned int subCACount;
+  const Result deferredSubjectError;
 
   Result RecordResult(Result currentResult, /*out*/ bool& keepGoing);
   Result result;
   bool resultWasSet;
 
   PathBuildingStep(const PathBuildingStep&) /*= delete*/;
   void operator=(const PathBuildingStep&) /*= delete*/;
 };
@@ -180,23 +182,29 @@ PathBuildingStep::Check(Input potentialI
   }
 
   rv = trustDomain.VerifySignedData(subject.GetSignedData(),
                                     potentialIssuer.GetSubjectPublicKeyInfo());
   if (rv != Success) {
     return RecordResult(rv, keepGoing);
   }
 
-  CertID certID(subject.GetIssuer(), potentialIssuer.GetSubjectPublicKeyInfo(),
-                subject.GetSerialNumber());
-  rv = trustDomain.CheckRevocation(subject.endEntityOrCA, certID, time,
-                                   stapledOCSPResponse,
-                                   subject.GetAuthorityInfoAccess());
-  if (rv != Success) {
-    return RecordResult(rv, keepGoing);
+  // We avoid doing revocation checking for expired certificates because OCSP
+  // responders are allowed to forget about expired certificates, and many OCSP
+  // responders return an error when asked for the status of an expired
+  // certificate.
+  if (deferredSubjectError != Result::ERROR_EXPIRED_CERTIFICATE) {
+    CertID certID(subject.GetIssuer(), potentialIssuer.GetSubjectPublicKeyInfo(),
+                  subject.GetSerialNumber());
+    rv = trustDomain.CheckRevocation(subject.endEntityOrCA, certID, time,
+                                     stapledOCSPResponse,
+                                     subject.GetAuthorityInfoAccess());
+    if (rv != Success) {
+      return RecordResult(rv, keepGoing);
+    }
   }
 
   return RecordResult(Success, keepGoing);
 }
 
 // Recursively build the path from the given subject certificate to the root.
 //
 // Be very careful about changing the order of checks. The order is significant
@@ -263,17 +271,18 @@ BuildForward(TrustDomain& trustDomain,
   } else {
     assert(subCACount == 0);
   }
 
   // Find a trusted issuer.
 
   PathBuildingStep pathBuilder(trustDomain, subject, time,
                                requiredEKUIfPresent, requiredPolicy,
-                               stapledOCSPResponse, subCACount);
+                               stapledOCSPResponse, subCACount,
+                               deferredEndEntityError);
 
   // TODO(bug 965136): Add SKI/AKI matching optimizations
   rv = trustDomain.FindIssuer(subject.GetIssuer(), pathBuilder, time);
   if (rv != Success) {
     return rv;
   }
 
   rv = pathBuilder.CheckResult();