Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1665715 - (2/2) pass encoded signed certificate timestamp extensi…
…on (if present) in CheckRevocation r=jcj

This will allow Firefox to make decisions based on the earliest known time that
a certificate exists (with respect to certificate transparency) that a CA is
unlikely to back-date. In particular, this is essential for CRLite. Note that
if the SCT signature isn't validated, a CA could still make a certificate
appear to have existed for longer than it really has. However, this change is
not an attempt to catch malicious CAs. The aim is to avoid false positives in
CRLite resulting from CAs backdating the notBefore field on certificates they
issue.

Depends on D90595

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

--HG--
extra : moz-landing-system : lando
  • Loading branch information
mozkeeler committed Sep 23, 2020
1 parent 6517a5c commit 33c0a6a
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 11 deletions.
18 changes: 12 additions & 6 deletions gtests/mozpkix_gtest/pkixbuild_tests.cpp
Expand Up @@ -153,7 +153,8 @@ class TestTrustDomain final : public DefaultCryptoTrustDomain
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*, /*optional*/ const Input*)
/*optional*/ const Input*, /*optional*/ const Input*,
/*optional*/ const Input*)
override
{
return Success;
Expand Down Expand Up @@ -302,7 +303,8 @@ class SingleRootTrustDomain : public DefaultCryptoTrustDomain
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*, /*optional*/ const Input*)
/*optional*/ const Input*, /*optional*/ const Input*,
/*optional*/ const Input*)
override
{
return Success;
Expand All @@ -322,7 +324,8 @@ class ExpiredCertTrustDomain final : public SingleRootTrustDomain
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*, /*optional*/ const Input*)
/*optional*/ const Input*, /*optional*/ const Input*,
/*optional*/ const Input*)
override
{
ADD_FAILURE();
Expand Down Expand Up @@ -443,7 +446,8 @@ class IssuerNameCheckTrustDomain final : public DefaultCryptoTrustDomain
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*, /*optional*/ const Input*)
/*optional*/ const Input*, /*optional*/ const Input*,
/*optional*/ const Input*)
override
{
return Success;
Expand Down Expand Up @@ -666,6 +670,7 @@ class MultiplePathTrustDomain: public DefaultCryptoTrustDomain
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*,
/*optional*/ const Input*,
/*optional*/ const Input*) override
{
Expand Down Expand Up @@ -724,7 +729,7 @@ class RevokedEndEntityTrustDomain final : public MultiplePathTrustDomain
public:
Result CheckRevocation(EndEntityOrCA endEntityOrCA, const CertID&, Time,
Duration, /*optional*/ const Input*,
/*optional*/ const Input*) override
/*optional*/ const Input*, /*optional*/ const Input*) override
{
if (endEntityOrCA == EndEntityOrCA::MustBeEndEntity) {
return Result::ERROR_REVOKED_CERTIFICATE;
Expand Down Expand Up @@ -829,7 +834,8 @@ class SelfIssuedCertificatesTrustDomain final : public DefaultCryptoTrustDomain
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*, /*optional*/ const Input*)
/*optional*/ const Input*, /*optional*/ const Input*,
/*optional*/ const Input*)
override
{
return Success;
Expand Down
3 changes: 2 additions & 1 deletion gtests/mozpkix_gtest/pkixcert_extension_tests.cpp
Expand Up @@ -71,7 +71,8 @@ class TrustEverythingTrustDomain final : public DefaultCryptoTrustDomain
}

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

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

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

Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*,
/*optional*/ const Input*,
/*optional*/ const Input*) override
{
Expand Down
1 change: 1 addition & 0 deletions gtests/mozpkix_gtest/pkixgtest.h
Expand Up @@ -101,6 +101,7 @@ class EverythingFailsByDefaultTrustDomain : public TrustDomain {
}

Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*,
/*optional*/ const Input*,
/*optional*/ const Input*) override {
ADD_FAILURE();
Expand Down
3 changes: 2 additions & 1 deletion lib/mozpkix/include/pkix/pkixtypes.h
Expand Up @@ -280,7 +280,8 @@ class TrustDomain {
const CertID& certID, Time time,
Duration validityDuration,
/*optional*/ const Input* stapledOCSPresponse,
/*optional*/ const Input* aiaExtension) = 0;
/*optional*/ const Input* aiaExtension,
/*optional*/ const Input* sctExtension) = 0;

// Check that the given digest algorithm is acceptable for use in signatures.
//
Expand Down
3 changes: 2 additions & 1 deletion lib/mozpkix/lib/pkixbuild.cpp
Expand Up @@ -253,7 +253,8 @@ PathBuildingStep::Check(Input potentialIssuerDER,
Duration validityDuration(notAfter, notBefore);
rv = trustDomain.CheckRevocation(subject.endEntityOrCA, certID, time,
validityDuration, stapledOCSPResponse,
subject.GetAuthorityInfoAccess());
subject.GetAuthorityInfoAccess(),
subject.GetSignedCertificateTimestamps());
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
Expand Down

0 comments on commit 33c0a6a

Please sign in to comment.