bug 1045739 - (1/2) mozilla::pkix: stop checking revocation for expired certificates r=keeler
authorBrian Smith <brian@briansmith.org>
Thu, 14 Aug 2014 12:02:55 -0700
changeset 208235 f7d98f2e35fd713eb2b2fc015ddd9acac13bd8b2
parent 208234 7719fd510b39c0461e2b20dba6b7c0cbae5fd737
child 208236 165c3fd176ec83c147d112717d105e9a12489ac4
push id27580
push userkwierso@gmail.com
push dateWed, 01 Oct 2014 23:26:55 +0000
treeherderautoland@af6c928893c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1045739
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1045739 - (1/2) mozilla::pkix: stop checking revocation for expired certificates r=keeler
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();