Bug 1592007 - land NSS e8f2720c8254 UPGRADE_NSS_RELEASE, r=kjacobs CLOSED TREE
authorJ.C. Jones <jc@mozilla.com>
Fri, 15 Nov 2019 18:08:09 +0100
changeset 502261 f3ce2609c3f30a4473054e4a76fd96e735c46176
parent 502260 cee8f503d3c32d97955c077405880439c6c2a365
child 502262 50366f885310c160e38ddafed53425e64d4acac1
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskjacobs
bugs1592007, 1593141
milestone72.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 1592007 - land NSS e8f2720c8254 UPGRADE_NSS_RELEASE, r=kjacobs CLOSED TREE 2019-11-09 Dana Keeler <dkeeler@mozilla.com> * gtests/mozpkix_gtest/pkixbuild_tests.cpp, gtests/mozpkix_gtest/pkixcert_extension_tests.cpp, gtests/mozpkix_gtest/pkixcert_signature_algorithm_tests.cpp, gtests/mozpkix_gtest/pkixcheck_CheckExtendedKeyUsage_tests.cpp, gtests/mozpkix_gtest/pkixcheck_CheckSignatureAlgorithm_tests.cpp, gtests/mozpkix_gtest/pkixgtest.h, lib/mozpkix/include/pkix/pkixtypes.h, lib/mozpkix/lib/pkixbuild.cpp: bug 1593141 - add validity period beginning argument to mozilla::pkix::TrustDomain::CheckRevocation r=jcj This allows TrustDomain implementations to make decisions based on when the validity period of a certificate began. For instance, if an implementation has revocation information that is valid and complete as of a particular time, but a certificate's validity period begins after that time, the implementation may decide to disregard this revocation information on the basis that the information it has available cannot possibly apply to that certificate. [e8f2720c8254] [tip] Differential Revision: https://phabricator.services.mozilla.com/D53228
security/nss/TAG-INFO
security/nss/coreconf/coreconf.dep
security/nss/gtests/mozpkix_gtest/pkixbuild_tests.cpp
security/nss/gtests/mozpkix_gtest/pkixcert_extension_tests.cpp
security/nss/gtests/mozpkix_gtest/pkixcert_signature_algorithm_tests.cpp
security/nss/gtests/mozpkix_gtest/pkixcheck_CheckExtendedKeyUsage_tests.cpp
security/nss/gtests/mozpkix_gtest/pkixcheck_CheckSignatureAlgorithm_tests.cpp
security/nss/gtests/mozpkix_gtest/pkixgtest.h
security/nss/lib/mozpkix/include/pkix/pkixtypes.h
security/nss/lib/mozpkix/lib/pkixbuild.cpp
--- a/security/nss/TAG-INFO
+++ b/security/nss/TAG-INFO
@@ -1,1 +1,1 @@
-87f35ba4c82f
\ No newline at end of file
+e8f2720c8254
\ No newline at end of file
--- a/security/nss/coreconf/coreconf.dep
+++ b/security/nss/coreconf/coreconf.dep
@@ -5,8 +5,9 @@
 
 /*
  * A dummy header file that is a dependency for all the object files.
  * Used to force a full recompilation of NSS in Mozilla's Tinderbox
  * depend builds.  See comments in rules.mk.
  */
 
 #error "Do not include this header file."
+
--- a/security/nss/gtests/mozpkix_gtest/pkixbuild_tests.cpp
+++ b/security/nss/gtests/mozpkix_gtest/pkixbuild_tests.cpp
@@ -147,20 +147,24 @@ private:
     rv = checker.Check(derCert, nullptr/*additionalNameConstraints*/,
                        keepGoing);
     if (rv != Success) {
       return rv;
     }
     return Success;
   }
 
-  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
+                         Time validityBeginning, Duration,
                          /*optional*/ const Input*, /*optional*/ const Input*)
                          override
   {
+    // All of the certificates in this test for which this is called have a
+    // validity period that begins "one day before now".
+    EXPECT_EQ(TimeFromEpochInSeconds(oneDayBeforeNow), validityBeginning);
     return Success;
   }
 
   Result IsChainValid(const DERArray&, Time, const CertPolicyId&) override
   {
     return Success;
   }
 
@@ -296,37 +300,41 @@ public:
     return checker.Check(rootCert, nullptr, keepGoing);
   }
 
   Result IsChainValid(const DERArray&, Time, const CertPolicyId&) override
   {
     return Success;
   }
 
-  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
+                         Time validityBeginning, Duration,
                          /*optional*/ const Input*, /*optional*/ const Input*)
                          override
   {
+    // All of the certificates in this test for which this is called have a
+    // validity period that begins "one day before now".
+    EXPECT_EQ(TimeFromEpochInSeconds(oneDayBeforeNow), validityBeginning);
     return Success;
   }
 
 private:
   ByteString rootDER;
 };
 
 // A TrustDomain that explicitly fails if CheckRevocation is called.
 class ExpiredCertTrustDomain final : public SingleRootTrustDomain
 {
 public:
   explicit ExpiredCertTrustDomain(ByteString aRootDER)
     : SingleRootTrustDomain(aRootDER)
   {
   }
 
-  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
                          /*optional*/ const Input*, /*optional*/ const Input*)
                          override
   {
     ADD_FAILURE();
     return NotReached("CheckRevocation should not be called",
                       Result::FATAL_ERROR_LIBRARY_FAILURE);
   }
 };
@@ -437,20 +445,24 @@ public:
     bool keepGoing;
     EXPECT_EQ(Success,
               checker.Check(issuerInput, nullptr /*additionalNameConstraints*/,
                             keepGoing));
     EXPECT_EQ(expectedKeepGoing, keepGoing);
     return Success;
   }
 
-  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
+                         Time validityBeginning, Duration,
                          /*optional*/ const Input*, /*optional*/ const Input*)
                          override
   {
+    // All of the certificates in this test for which this is called have a
+    // validity period that begins "one day before now".
+    EXPECT_EQ(TimeFromEpochInSeconds(oneDayBeforeNow), validityBeginning);
     return Success;
   }
 
   Result IsChainValid(const DERArray&, Time, const CertPolicyId&) override
   {
     return Success;
   }
 
@@ -660,20 +672,24 @@ private:
                      keepGoing);
       if (rv != Success) {
         return rv;
       }
     }
     return Success;
   }
 
-  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
+                         Time validityBeginning, Duration,
                          /*optional*/ const Input*,
                          /*optional*/ const Input*) override
   {
+    // All of the certificates in this test for which this is called have a
+    // validity period that begins "one day before now".
+    EXPECT_EQ(TimeFromEpochInSeconds(oneDayBeforeNow), validityBeginning);
     return Success;
   }
 
   Result IsChainValid(const DERArray&, Time, const CertPolicyId&) override
   {
     return Success;
   }
 
@@ -718,17 +734,17 @@ TEST_F(pkixbuild, BadEmbeddedSCTWithMult
                            nullptr/*stapledOCSPResponse*/));
 }
 
 // Same as a MultiplePathTrustDomain, but the end-entity is revoked.
 class RevokedEndEntityTrustDomain final : public MultiplePathTrustDomain
 {
 public:
   Result CheckRevocation(EndEntityOrCA endEntityOrCA, const CertID&, Time,
-                         Duration, /*optional*/ const Input*,
+                         Time, Duration, /*optional*/ const Input*,
                          /*optional*/ const Input*) override
   {
     if (endEntityOrCA == EndEntityOrCA::MustBeEndEntity) {
       return Result::ERROR_REVOKED_CERTIFICATE;
     }
     return Success;
   }
 };
@@ -823,20 +839,24 @@ private:
       rv = checker.Check(certInput, nullptr, keepGoing);
       if (rv != Success || !keepGoing) {
         return rv;
       }
     }
     return Success;
   }
 
-  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
+                         Time validityBeginning, Duration,
                          /*optional*/ const Input*, /*optional*/ const Input*)
                          override
   {
+    // All of the certificates in this test for which this is called have a
+    // validity period that begins "one day before now".
+    EXPECT_EQ(TimeFromEpochInSeconds(oneDayBeforeNow), validityBeginning);
     return Success;
   }
 
   Result IsChainValid(const DERArray&, Time, const CertPolicyId&) override
   {
     return Success;
   }
 
--- a/security/nss/gtests/mozpkix_gtest/pkixcert_extension_tests.cpp
+++ b/security/nss/gtests/mozpkix_gtest/pkixcert_extension_tests.cpp
@@ -65,17 +65,17 @@ class TrustEverythingTrustDomain final :
 private:
   Result GetCertTrust(EndEntityOrCA, const CertPolicyId&, Input,
                       /*out*/ TrustLevel& trustLevel) override
   {
     trustLevel = TrustLevel::TrustAnchor;
     return Success;
   }
 
-  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
                          /*optional*/ const Input*, /*optional*/ const Input*)
                          override
   {
     return Success;
   }
 
   Result IsChainValid(const DERArray&, Time, const CertPolicyId&) override
   {
--- a/security/nss/gtests/mozpkix_gtest/pkixcert_signature_algorithm_tests.cpp
+++ b/security/nss/gtests/mozpkix_gtest/pkixcert_signature_algorithm_tests.cpp
@@ -87,17 +87,17 @@ private:
     Result rv = issuerCert.Init(issuerDER->data(), issuerDER->length());
     if (rv != Success) {
       return rv;
     }
     bool keepGoing;
     return checker.Check(issuerCert, nullptr, keepGoing);
   }
 
-  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
                          const Input*, const Input*) override
   {
     return Success;
   }
 
   Result IsChainValid(const DERArray&, Time, const CertPolicyId&) override
   {
     return Success;
--- a/security/nss/gtests/mozpkix_gtest/pkixcheck_CheckExtendedKeyUsage_tests.cpp
+++ b/security/nss/gtests/mozpkix_gtest/pkixcheck_CheckExtendedKeyUsage_tests.cpp
@@ -553,17 +553,17 @@ private:
     Result rv = derCert.Init(mIssuerCertDER.data(), mIssuerCertDER.length());
     if (rv != Success) {
       return rv;
     }
     bool keepGoing;
     return checker.Check(derCert, nullptr, keepGoing);
   }
 
-  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
                          const Input*, const Input*) override
   {
     return Success;
   }
 
   Result IsChainValid(const DERArray&, Time, const CertPolicyId&) override
   {
     return Success;
--- a/security/nss/gtests/mozpkix_gtest/pkixcheck_CheckSignatureAlgorithm_tests.cpp
+++ b/security/nss/gtests/mozpkix_gtest/pkixcheck_CheckSignatureAlgorithm_tests.cpp
@@ -297,17 +297,17 @@ public:
 
     bool keepGoing;
     EXPECT_EQ(Success, checker.Check(issuerInput, nullptr, keepGoing));
     EXPECT_FALSE(keepGoing);
 
     return Success;
   }
 
-  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
                          /*optional*/ const Input*,
                          /*optional*/ const Input*) override
   {
     return Success;
   }
 
   Result IsChainValid(const DERArray&, Time, const CertPolicyId&) override
   {
--- a/security/nss/gtests/mozpkix_gtest/pkixgtest.h
+++ b/security/nss/gtests/mozpkix_gtest/pkixgtest.h
@@ -95,17 +95,17 @@ class EverythingFailsByDefaultTrustDomai
   }
 
   Result FindIssuer(Input, IssuerChecker&, Time) override {
     ADD_FAILURE();
     return NotReached("FindIssuer should not be called",
                       Result::FATAL_ERROR_LIBRARY_FAILURE);
   }
 
-  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
                          /*optional*/ const Input*,
                          /*optional*/ const Input*) override {
     ADD_FAILURE();
     return NotReached("CheckRevocation should not be called",
                       Result::FATAL_ERROR_LIBRARY_FAILURE);
   }
 
   Result IsChainValid(const DERArray&, Time, const CertPolicyId&) override {
--- a/security/nss/lib/mozpkix/include/pkix/pkixtypes.h
+++ b/security/nss/lib/mozpkix/include/pkix/pkixtypes.h
@@ -273,16 +273,17 @@ class TrustDomain {
   // wrong to assume that the certificate chain is valid.
   //
   // certChain.GetDER(0) is the trust anchor.
   virtual Result IsChainValid(const DERArray& certChain, Time time,
                               const CertPolicyId& requiredPolicy) = 0;
 
   virtual Result CheckRevocation(EndEntityOrCA endEntityOrCA,
                                  const CertID& certID, Time time,
+                                 Time validityBeginning,
                                  Duration validityDuration,
                                  /*optional*/ const Input* stapledOCSPresponse,
                                  /*optional*/ const Input* aiaExtension) = 0;
 
   // Check that the given digest algorithm is acceptable for use in signatures.
   //
   // Return Success if the algorithm is acceptable,
   // Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED if the algorithm is not
--- a/security/nss/lib/mozpkix/lib/pkixbuild.cpp
+++ b/security/nss/lib/mozpkix/lib/pkixbuild.cpp
@@ -247,17 +247,18 @@ PathBuildingStep::Check(Input potentialI
     // This should never fail. If we're here, we've already parsed the validity
     // and checked that the given time is in the certificate's validity period.
     rv = ParseValidity(subject.GetValidity(), &notBefore, &notAfter);
     if (rv != Success) {
       return rv;
     }
     Duration validityDuration(notAfter, notBefore);
     rv = trustDomain.CheckRevocation(subject.endEntityOrCA, certID, time,
-                                     validityDuration, stapledOCSPResponse,
+                                     notBefore, validityDuration,
+                                     stapledOCSPResponse,
                                      subject.GetAuthorityInfoAccess());
     if (rv != Success) {
       // Since this is actually a problem with the current subject certificate
       // (rather than the issuer), it doesn't make sense to keep going; all
       // paths through this certificate will fail.
       Result savedRv = RecordResult(rv, keepGoing);
       keepGoing = false;
       return savedRv;