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. Differential Revision: https://phabricator.services.mozilla.com/D51485 --HG-- extra : moz-landing-system : lando
sailfishos-mirror · Nov 9, 2019 · cab26a4 · cab26a4
1 parent b420c6b
commit cab26a4
Show file tree

Hide file tree

Showing 8 changed files with 35 additions and 13 deletions.
diff --git a/gtests/mozpkix_gtest/pkixbuild_tests.cpp b/gtests/mozpkix_gtest/pkixbuild_tests.cpp
@@ -152,10 +152,14 @@ class TestTrustDomain final : public DefaultCryptoTrustDomain
     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;
   }
 
@@ -301,10 +305,14 @@ class SingleRootTrustDomain : public DefaultCryptoTrustDomain
     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;
   }
 
@@ -321,7 +329,7 @@ class ExpiredCertTrustDomain final : public SingleRootTrustDomain
   {
   }
 
-  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
                          /*optional*/ const Input*, /*optional*/ const Input*)
                          override
   {
@@ -442,10 +450,14 @@ class IssuerNameCheckTrustDomain final : public DefaultCryptoTrustDomain
     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;
   }
 
@@ -665,10 +677,14 @@ class MultiplePathTrustDomain: public DefaultCryptoTrustDomain
     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;
   }
 
@@ -723,7 +739,7 @@ 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) {
@@ -828,10 +844,14 @@ class SelfIssuedCertificatesTrustDomain final : public DefaultCryptoTrustDomain
     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;
   }
 

diff --git a/gtests/mozpkix_gtest/pkixcert_extension_tests.cpp b/gtests/mozpkix_gtest/pkixcert_extension_tests.cpp
@@ -70,7 +70,7 @@ class TrustEverythingTrustDomain final : public DefaultCryptoTrustDomain
     return Success;
   }
 
-  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
                          /*optional*/ const Input*, /*optional*/ const Input*)
                          override
   {

diff --git a/gtests/mozpkix_gtest/pkixcert_signature_algorithm_tests.cpp b/gtests/mozpkix_gtest/pkixcert_signature_algorithm_tests.cpp
@@ -92,7 +92,7 @@ class AlgorithmTestsTrustDomain final : public DefaultCryptoTrustDomain
     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;

diff --git a/gtests/mozpkix_gtest/pkixcheck_CheckExtendedKeyUsage_tests.cpp b/gtests/mozpkix_gtest/pkixcheck_CheckExtendedKeyUsage_tests.cpp
@@ -558,7 +558,7 @@ class EKUTrustDomain final : public DefaultCryptoTrustDomain
     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;

diff --git a/gtests/mozpkix_gtest/pkixcheck_CheckSignatureAlgorithm_tests.cpp b/gtests/mozpkix_gtest/pkixcheck_CheckSignatureAlgorithm_tests.cpp
@@ -302,7 +302,7 @@ class pkixcheck_CheckSignatureAlgorithm_BuildCertChain_TrustDomain
     return Success;
   }
 
-  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
                          /*optional*/ const Input*,
                          /*optional*/ const Input*) override
   {

diff --git a/gtests/mozpkix_gtest/pkixgtest.h b/gtests/mozpkix_gtest/pkixgtest.h
@@ -100,7 +100,7 @@ class EverythingFailsByDefaultTrustDomain : public TrustDomain {
                       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();

diff --git a/lib/mozpkix/include/pkix/pkixtypes.h b/lib/mozpkix/include/pkix/pkixtypes.h
@@ -278,6 +278,7 @@ class TrustDomain {
 
   virtual Result CheckRevocation(EndEntityOrCA endEntityOrCA,
                                  const CertID& certID, Time time,
+                                 Time validityBeginning,
                                  Duration validityDuration,
                                  /*optional*/ const Input* stapledOCSPresponse,
                                  /*optional*/ const Input* aiaExtension) = 0;

diff --git a/lib/mozpkix/lib/pkixbuild.cpp b/lib/mozpkix/lib/pkixbuild.cpp
@@ -252,7 +252,8 @@ PathBuildingStep::Check(Input potentialIssuerDER,
     }
     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