Backed out changesets 013c6a8663ab and 5f8dea9b01ac (bug 1045739) for B2G bustage.
authorRyan VanderMeulen <ryanvm@gmail.com>
Tue, 07 Oct 2014 19:54:37 -0400
changeset 223084 49bd5bad84d2f69bd89cc50dcb22639637fd9800
parent 223083 32f99eac065c37b349779da239c0731b117f8c92
child 223085 a4933ddd6a91af1475eac62db03fa24182d6fae9
push id4
push usergszorc@mozilla.com
push dateWed, 29 Oct 2014 02:48:29 +0000
bugs1045739
milestone34.0a2
backs out013c6a8663ab815dbb2f4d13cf42934b57788a3b
5f8dea9b01ac259dc0463c2fecc6f8a13834bf6f
Backed out changesets 013c6a8663ab and 5f8dea9b01ac (bug 1045739) for B2G bustage.
security/pkix/lib/pkixbuild.cpp
security/pkix/test/gtest/pkixbuild_tests.cpp
--- a/security/pkix/lib/pkixbuild.cpp
+++ b/security/pkix/lib/pkixbuild.cpp
@@ -45,25 +45,24 @@ 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, Result deferredSubjectError)
+                   unsigned int subCACount)
     : 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);
@@ -73,17 +72,16 @@ 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*/;
 };
@@ -182,29 +180,23 @@ PathBuildingStep::Check(Input potentialI
   }
 
   rv = trustDomain.VerifySignedData(subject.GetSignedData(),
                                     potentialIssuer.GetSubjectPublicKeyInfo());
   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);
-    }
+  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
@@ -271,18 +263,17 @@ BuildForward(TrustDomain& trustDomain,
   } else {
     assert(subCACount == 0);
   }
 
   // Find a trusted issuer.
 
   PathBuildingStep pathBuilder(trustDomain, subject, time,
                                requiredEKUIfPresent, requiredPolicy,
-                               stapledOCSPResponse, subCACount,
-                               deferredEndEntityError);
+                               stapledOCSPResponse, subCACount);
 
   // TODO(bug 965136): Add SKI/AKI matching optimizations
   rv = trustDomain.FindIssuer(subject.GetIssuer(), pathBuilder, time);
   if (rv != Success) {
     return rv;
   }
 
   rv = pathBuilder.CheckResult();
--- a/security/pkix/test/gtest/pkixbuild_tests.cpp
+++ b/security/pkix/test/gtest/pkixbuild_tests.cpp
@@ -292,129 +292,8 @@ TEST_F(pkixbuild, BeyondMaxAcceptableCer
               BuildCertChain(trustDomain, cert, Now(),
                              EndEntityOrCA::MustBeEndEntity,
                              KeyUsage::noParticularKeyUsageRequired,
                              KeyPurposeId::id_kp_serverAuth,
                              CertPolicyId::anyPolicy,
                              nullptr/*stapledOCSPResponse*/));
   }
 }
-
-// A TrustDomain that explicitly fails if CheckRevocation is called.
-// It is initialized with the DER encoding of a root certificate that
-// is treated as a trust anchor and is assumed to have issued all certificates
-// (i.e. FindIssuer always attempts to build the next step in the chain with
-// it).
-class ExpiredCertTrustDomain : public TrustDomain
-{
-public:
-  ExpiredCertTrustDomain(CERTCertificate* rootCert)
-    : rootCert(rootCert)
-  {
-  }
-
-  // The CertPolicyId argument is unused because we don't care about EV.
-  virtual Result GetCertTrust(EndEntityOrCA, const CertPolicyId&,
-                              Input candidateCert,
-                              /*out*/ TrustLevel& trustLevel)
-  {
-    Input rootDER;
-    Result rv = rootDER.Init(rootCert->derCert.data,
-                             rootCert->derCert.len);
-    EXPECT_EQ(Success, rv);
-    if (InputsAreEqual(candidateCert, rootDER)) {
-      trustLevel = TrustLevel::TrustAnchor;
-    } else {
-      trustLevel = TrustLevel::InheritsTrust;
-    }
-    return Success;
-  }
-
-  virtual Result FindIssuer(Input encodedIssuerName,
-                            IssuerChecker& checker, Time time)
-  {
-    Input derCert;
-    Result rv = derCert.Init(rootCert->derCert.data, rootCert->derCert.len);
-    EXPECT_EQ(Success, rv);
-    if (rv != Success) {
-      return rv;
-    }
-    // keepGoing is an out parameter from IssuerChecker.Check. It would tell us
-    // whether or not to continue attempting other potential issuers. We only
-    // know of one potential issuer, however, so we ignore it.
-    bool keepGoing;
-    return checker.Check(derCert, nullptr, keepGoing);
-  }
-
-  virtual Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
-                                 /*optional*/ const Input*,
-                                 /*optional*/ const Input*)
-  {
-    ADD_FAILURE();
-    return Result::FATAL_ERROR_LIBRARY_FAILURE;
-  }
-
-  virtual Result IsChainValid(const DERArray&)
-  {
-    return Success;
-  }
-
-  virtual Result VerifySignedData(const SignedDataWithSignature& signedData,
-                                  Input subjectPublicKeyInfo)
-  {
-    return ::mozilla::pkix::VerifySignedData(signedData, subjectPublicKeyInfo,
-                                             nullptr);
-  }
-
-  virtual Result DigestBuf(Input, uint8_t*, size_t)
-  {
-    ADD_FAILURE();
-    return Result::FATAL_ERROR_LIBRARY_FAILURE;
-  }
-
-  virtual Result CheckPublicKey(Input subjectPublicKeyInfo)
-  {
-    return ::mozilla::pkix::CheckPublicKey(subjectPublicKeyInfo);
-  }
-
-private:
-  ScopedCERTCertificate rootCert;
-};
-
-TEST_F(pkixbuild, NoRevocationCheckingForExpiredCert)
-{
-  ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
-  ASSERT_NE(nullptr, arena.get());
-
-  const char* rootCN = "CN=Root CA";
-  ScopedSECKEYPrivateKey rootKey;
-  ScopedCERTCertificate rootCert;
-  (void) CreateCert(arena.get(), rootCN, rootCN, EndEntityOrCA::MustBeCA,
-                    nullptr, rootKey, &rootCert);
-  ExpiredCertTrustDomain expiredCertTrustDomain(rootCert.release());
-
-  const SECItem* serialNumber(CreateEncodedSerialNumber(arena.get(), 100));
-  EXPECT_NE(nullptr, serialNumber);
-  const SECItem* issuerDER(ASCIIToDERName(arena.get(), rootCN));
-  EXPECT_NE(nullptr, issuerDER);
-  const SECItem* subjectDER(ASCIIToDERName(arena.get(), "CN=Expired End-Entity Cert"));
-  EXPECT_NE(nullptr, subjectDER);
-  ScopedSECKEYPrivateKey unusedSubjectKey;
-  SECItem* certDER(CreateEncodedCertificate(
-                     arena.get(), v3,
-                     SEC_OID_PKCS1_SHA256_WITH_RSA_ENCRYPTION,
-                     serialNumber, issuerDER,
-                     oneDayBeforeNow - Time::ONE_DAY_IN_SECONDS,
-                     oneDayBeforeNow,
-                     subjectDER, nullptr, rootKey.get(),
-                     SEC_OID_SHA256,
-                     unusedSubjectKey));
-  EXPECT_NE(nullptr, certDER);
-  Input certInput;
-  EXPECT_EQ(Success, certInput.Init(certDER->data, certDER->len));
-  ASSERT_EQ(Result::ERROR_EXPIRED_CERTIFICATE,
-            BuildCertChain(expiredCertTrustDomain, certInput,
-                           Now(), EndEntityOrCA::MustBeEndEntity,
-                           KeyUsage::noParticularKeyUsageRequired,
-                           KeyPurposeId::id_kp_serverAuth,
-                           CertPolicyId::anyPolicy,
-                           nullptr));
-}