Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1665715 - (1/2) revert e8f2720c8254 (bug 1593141) because it's no…
… longer necessary r=jcj

Bug 1593141 added the certificate's notBefore field as an argument to
TrustDomain::CheckRevocation so that Firefox could use it with CRLite.
However, since CAs can backdate that field, we need to use the earliest
embedded SCT timestamp instead.

Differential Revision: https://phabricator.services.mozilla.com/D90595

--HG--
extra : moz-landing-system : lando
  • Loading branch information
mozkeeler committed Sep 18, 2020
1 parent 5ca6c53 commit 6517a5c
Show file tree
Hide file tree
Showing 8 changed files with 13 additions and 35 deletions.
34 changes: 7 additions & 27 deletions gtests/mozpkix_gtest/pkixbuild_tests.cpp
Expand Up @@ -152,14 +152,10 @@ class TestTrustDomain final : public DefaultCryptoTrustDomain
return Success;
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
Time validityBeginning, Duration,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, 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;
}

Expand Down Expand Up @@ -305,14 +301,10 @@ class SingleRootTrustDomain : public DefaultCryptoTrustDomain
return Success;
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
Time validityBeginning, Duration,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, 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;
}

Expand All @@ -329,7 +321,7 @@ class ExpiredCertTrustDomain final : public SingleRootTrustDomain
{
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*, /*optional*/ const Input*)
override
{
Expand Down Expand Up @@ -450,14 +442,10 @@ class IssuerNameCheckTrustDomain final : public DefaultCryptoTrustDomain
return Success;
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
Time validityBeginning, Duration,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, 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;
}

Expand Down Expand Up @@ -677,14 +665,10 @@ class MultiplePathTrustDomain: public DefaultCryptoTrustDomain
return Success;
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
Time validityBeginning, Duration,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, 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;
}

Expand Down Expand Up @@ -739,7 +723,7 @@ class RevokedEndEntityTrustDomain final : public MultiplePathTrustDomain
{
public:
Result CheckRevocation(EndEntityOrCA endEntityOrCA, const CertID&, Time,
Time, Duration, /*optional*/ const Input*,
Duration, /*optional*/ const Input*,
/*optional*/ const Input*) override
{
if (endEntityOrCA == EndEntityOrCA::MustBeEndEntity) {
Expand Down Expand Up @@ -844,14 +828,10 @@ class SelfIssuedCertificatesTrustDomain final : public DefaultCryptoTrustDomain
return Success;
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
Time validityBeginning, Duration,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, 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;
}

Expand Down
2 changes: 1 addition & 1 deletion gtests/mozpkix_gtest/pkixcert_extension_tests.cpp
Expand Up @@ -70,7 +70,7 @@ class TrustEverythingTrustDomain final : public DefaultCryptoTrustDomain
return Success;
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*, /*optional*/ const Input*)
override
{
Expand Down
Expand Up @@ -92,7 +92,7 @@ class AlgorithmTestsTrustDomain final : public DefaultCryptoTrustDomain
return checker.Check(issuerCert, nullptr, keepGoing);
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
const Input*, const Input*) override
{
return Success;
Expand Down
Expand Up @@ -558,7 +558,7 @@ class EKUTrustDomain final : public DefaultCryptoTrustDomain
return checker.Check(derCert, nullptr, keepGoing);
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
const Input*, const Input*) override
{
return Success;
Expand Down
Expand Up @@ -302,7 +302,7 @@ class pkixcheck_CheckSignatureAlgorithm_BuildCertChain_TrustDomain
return Success;
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*,
/*optional*/ const Input*) override
{
Expand Down
2 changes: 1 addition & 1 deletion gtests/mozpkix_gtest/pkixgtest.h
Expand Up @@ -100,7 +100,7 @@ class EverythingFailsByDefaultTrustDomain : public TrustDomain {
Result::FATAL_ERROR_LIBRARY_FAILURE);
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*,
/*optional*/ const Input*) override {
ADD_FAILURE();
Expand Down
1 change: 0 additions & 1 deletion lib/mozpkix/include/pkix/pkixtypes.h
Expand Up @@ -278,7 +278,6 @@ 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;
Expand Down
3 changes: 1 addition & 2 deletions lib/mozpkix/lib/pkixbuild.cpp
Expand Up @@ -252,8 +252,7 @@ PathBuildingStep::Check(Input potentialIssuerDER,
}
Duration validityDuration(notAfter, notBefore);
rv = trustDomain.CheckRevocation(subject.endEntityOrCA, certID, time,
notBefore, validityDuration,
stapledOCSPResponse,
validityDuration, stapledOCSPResponse,
subject.GetAuthorityInfoAccess());
if (rv != Success) {
// Since this is actually a problem with the current subject certificate
Expand Down

0 comments on commit 6517a5c

Please sign in to comment.