bug 1141189 - implement skipping expensive revocation checks (OCSP fetching) for short-lived certificates r=rbarnes

This commit is contained in:
David Keeler 2015-04-06 16:10:28 -07:00
parent 935edace34
commit 77060a5e28
20 changed files with 165 additions and 44 deletions

View File

@ -234,7 +234,7 @@ AppTrustDomain::DigestBuf(Input item,
}
Result
AppTrustDomain::CheckRevocation(EndEntityOrCA, const CertID&, Time,
AppTrustDomain::CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*,
/*optional*/ const Input*)
{

View File

@ -34,6 +34,7 @@ public:
virtual Result CheckRevocation(mozilla::pkix::EndEntityOrCA endEntityOrCA,
const mozilla::pkix::CertID& certID,
mozilla::pkix::Time time,
mozilla::pkix::Duration validityDuration,
/*optional*/ const mozilla::pkix::Input* stapledOCSPresponse,
/*optional*/ const mozilla::pkix::Input* aiaExtension) override;
virtual Result IsChainValid(const mozilla::pkix::DERArray& certChain,

View File

@ -32,10 +32,12 @@ const CertVerifier::Flags CertVerifier::FLAG_MUST_BE_EV = 2;
CertVerifier::CertVerifier(OcspDownloadConfig odc,
OcspStrictConfig osc,
OcspGetConfig ogc,
uint32_t certShortLifetimeInDays,
PinningMode pinningMode)
: mOCSPDownloadEnabled(odc == ocspOn)
, mOCSPStrict(osc == ocspStrict)
, mOCSPGETEnabled(ogc == ocspGetEnabled)
, mCertShortLifetimeInDays(certShortLifetimeInDays)
, mPinningMode(pinningMode)
{
}
@ -193,7 +195,9 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage,
// XXX: We don't really have a trust bit for SSL client authentication so
// just use trustEmail as it is the closest alternative.
NSSCertDBTrustDomain trustDomain(trustEmail, ocspFetching, mOCSPCache,
pinArg, ocspGETConfig, pinningDisabled,
pinArg, ocspGETConfig,
mCertShortLifetimeInDays,
pinningDisabled,
MIN_RSA_BITS_WEAK, nullptr, builtChain);
rv = BuildCertChain(trustDomain, certDER, time,
EndEntityOrCA::MustBeEndEntity,
@ -219,8 +223,9 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage,
ocspFetching == NSSCertDBTrustDomain::NeverFetchOCSP
? NSSCertDBTrustDomain::LocalOnlyOCSPForEV
: NSSCertDBTrustDomain::FetchOCSPForEV,
mOCSPCache, pinArg, ocspGETConfig, mPinningMode,
MIN_RSA_BITS, hostname, builtChain);
mOCSPCache, pinArg, ocspGETConfig,
mCertShortLifetimeInDays, mPinningMode, MIN_RSA_BITS,
hostname, builtChain);
rv = BuildCertChainForOneKeyUsage(trustDomain, certDER, time,
KeyUsage::digitalSignature,// (EC)DHE
KeyUsage::keyEncipherment, // RSA
@ -244,7 +249,8 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage,
// Now try non-EV.
NSSCertDBTrustDomain trustDomain(trustSSL, ocspFetching, mOCSPCache,
pinArg, ocspGETConfig, mPinningMode,
pinArg, ocspGETConfig,
mCertShortLifetimeInDays, mPinningMode,
MIN_RSA_BITS, hostname, builtChain);
rv = BuildCertChainForOneKeyUsage(trustDomain, certDER, time,
KeyUsage::digitalSignature, // (EC)DHE
@ -263,9 +269,10 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage,
// If that failed, try again with a smaller minimum key size.
NSSCertDBTrustDomain trustDomainWeak(trustSSL, ocspFetching, mOCSPCache,
pinArg, ocspGETConfig, mPinningMode,
MIN_RSA_BITS_WEAK, hostname,
builtChain);
pinArg, ocspGETConfig,
mCertShortLifetimeInDays,
mPinningMode, MIN_RSA_BITS_WEAK,
hostname, builtChain);
rv = BuildCertChainForOneKeyUsage(trustDomainWeak, certDER, time,
KeyUsage::digitalSignature, // (EC)DHE
KeyUsage::keyEncipherment, // RSA
@ -287,8 +294,10 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage,
case certificateUsageSSLCA: {
NSSCertDBTrustDomain trustDomain(trustSSL, ocspFetching, mOCSPCache,
pinArg, ocspGETConfig, pinningDisabled,
MIN_RSA_BITS_WEAK, nullptr, builtChain);
pinArg, ocspGETConfig,
mCertShortLifetimeInDays,
pinningDisabled, MIN_RSA_BITS_WEAK,
nullptr, builtChain);
rv = BuildCertChain(trustDomain, certDER, time,
EndEntityOrCA::MustBeCA, KeyUsage::keyCertSign,
KeyPurposeId::id_kp_serverAuth,
@ -298,8 +307,10 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage,
case certificateUsageEmailSigner: {
NSSCertDBTrustDomain trustDomain(trustEmail, ocspFetching, mOCSPCache,
pinArg, ocspGETConfig, pinningDisabled,
MIN_RSA_BITS_WEAK, nullptr, builtChain);
pinArg, ocspGETConfig,
mCertShortLifetimeInDays,
pinningDisabled, MIN_RSA_BITS_WEAK,
nullptr, builtChain);
rv = BuildCertChain(trustDomain, certDER, time,
EndEntityOrCA::MustBeEndEntity,
KeyUsage::digitalSignature,
@ -320,8 +331,10 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage,
// usage it is trying to verify for, and base its algorithm choices
// based on the result of the verification(s).
NSSCertDBTrustDomain trustDomain(trustEmail, ocspFetching, mOCSPCache,
pinArg, ocspGETConfig, pinningDisabled,
MIN_RSA_BITS_WEAK, nullptr, builtChain);
pinArg, ocspGETConfig,
mCertShortLifetimeInDays,
pinningDisabled, MIN_RSA_BITS_WEAK,
nullptr, builtChain);
rv = BuildCertChain(trustDomain, certDER, time,
EndEntityOrCA::MustBeEndEntity,
KeyUsage::keyEncipherment, // RSA
@ -340,8 +353,9 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage,
case certificateUsageObjectSigner: {
NSSCertDBTrustDomain trustDomain(trustObjectSigning, ocspFetching,
mOCSPCache, pinArg, ocspGETConfig,
pinningDisabled,
MIN_RSA_BITS_WEAK, nullptr, builtChain);
mCertShortLifetimeInDays,
pinningDisabled, MIN_RSA_BITS_WEAK,
nullptr, builtChain);
rv = BuildCertChain(trustDomain, certDER, time,
EndEntityOrCA::MustBeEndEntity,
KeyUsage::digitalSignature,
@ -369,15 +383,18 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage,
}
NSSCertDBTrustDomain sslTrust(trustSSL, ocspFetching, mOCSPCache, pinArg,
ocspGETConfig, pinningDisabled,
MIN_RSA_BITS_WEAK, nullptr, builtChain);
ocspGETConfig, mCertShortLifetimeInDays,
pinningDisabled, MIN_RSA_BITS_WEAK,
nullptr, builtChain);
rv = BuildCertChain(sslTrust, certDER, time, endEntityOrCA,
keyUsage, eku, CertPolicyId::anyPolicy,
stapledOCSPResponse);
if (rv == Result::ERROR_UNKNOWN_ISSUER) {
NSSCertDBTrustDomain emailTrust(trustEmail, ocspFetching, mOCSPCache,
pinArg, ocspGETConfig, pinningDisabled,
MIN_RSA_BITS_WEAK, nullptr, builtChain);
pinArg, ocspGETConfig,
mCertShortLifetimeInDays,
pinningDisabled, MIN_RSA_BITS_WEAK,
nullptr, builtChain);
rv = BuildCertChain(emailTrust, certDER, time, endEntityOrCA,
keyUsage, eku, CertPolicyId::anyPolicy,
stapledOCSPResponse);
@ -385,6 +402,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage,
NSSCertDBTrustDomain objectSigningTrust(trustObjectSigning,
ocspFetching, mOCSPCache,
pinArg, ocspGETConfig,
mCertShortLifetimeInDays,
pinningDisabled,
MIN_RSA_BITS_WEAK,
nullptr, builtChain);

View File

@ -82,7 +82,8 @@ public:
bool IsOCSPDownloadEnabled() const { return mOCSPDownloadEnabled; }
CertVerifier(OcspDownloadConfig odc, OcspStrictConfig osc,
OcspGetConfig ogc, PinningMode pinningMode);
OcspGetConfig ogc, uint32_t certShortLifetimeInDays,
PinningMode pinningMode);
~CertVerifier();
void ClearOCSPCache() { mOCSPCache.Clear(); }
@ -90,6 +91,7 @@ public:
const bool mOCSPDownloadEnabled;
const bool mOCSPStrict;
const bool mOCSPGETEnabled;
const uint32_t mCertShortLifetimeInDays;
const PinningMode mPinningMode;
private:

View File

@ -45,6 +45,7 @@ NSSCertDBTrustDomain::NSSCertDBTrustDomain(SECTrustType certDBTrustType,
OCSPCache& ocspCache,
/*optional but shouldn't be*/ void* pinArg,
CertVerifier::OcspGetConfig ocspGETConfig,
uint32_t certShortLifetimeInDays,
CertVerifier::PinningMode pinningMode,
unsigned int minRSABits,
/*optional*/ const char* hostname,
@ -54,6 +55,7 @@ NSSCertDBTrustDomain::NSSCertDBTrustDomain(SECTrustType certDBTrustType,
, mOCSPCache(ocspCache)
, mPinArg(pinArg)
, mOCSPGetConfig(ocspGETConfig)
, mCertShortLifetimeInDays(certShortLifetimeInDays)
, mPinningMode(pinningMode)
, mMinRSABits(minRSABits)
, mHostname(hostname)
@ -329,6 +331,7 @@ GetOCSPAuthorityInfoAccessLocation(PLArenaPool* arena,
Result
NSSCertDBTrustDomain::CheckRevocation(EndEntityOrCA endEntityOrCA,
const CertID& certID, Time time,
Duration validityDuration,
/*optional*/ const Input* stapledOCSPResponse,
/*optional*/ const Input* aiaExtension)
{
@ -455,7 +458,10 @@ NSSCertDBTrustDomain::CheckRevocation(EndEntityOrCA endEntityOrCA,
// security.OCSP.enable==0 means "I want the default" or "I really never want
// you to ever fetch OCSP."
Duration shortLifetime(mCertShortLifetimeInDays * Time::ONE_DAY_IN_SECONDS);
if ((mOCSPFetching == NeverFetchOCSP) ||
(validityDuration < shortLifetime) ||
(endEntityOrCA == EndEntityOrCA::MustBeCA &&
(mOCSPFetching == FetchOCSPForDVHardFail ||
mOCSPFetching == FetchOCSPForDVSoftFail ||

View File

@ -53,6 +53,7 @@ public:
NSSCertDBTrustDomain(SECTrustType certDBTrustType, OCSPFetching ocspFetching,
OCSPCache& ocspCache, void* pinArg,
CertVerifier::OcspGetConfig ocspGETConfig,
uint32_t certShortLifetimeInDays,
CertVerifier::PinningMode pinningMode,
unsigned int minRSABits,
/*optional*/ const char* hostname = nullptr,
@ -96,6 +97,7 @@ public:
mozilla::pkix::EndEntityOrCA endEntityOrCA,
const mozilla::pkix::CertID& certID,
mozilla::pkix::Time time,
mozilla::pkix::Duration validityDuration,
/*optional*/ const mozilla::pkix::Input* stapledOCSPResponse,
/*optional*/ const mozilla::pkix::Input* aiaExtension)
override;
@ -127,6 +129,7 @@ private:
OCSPCache& mOCSPCache; // non-owning!
void* mPinArg; // non-owning!
const CertVerifier::OcspGetConfig mOCSPGetConfig;
const uint32_t mCertShortLifetimeInDays;
CertVerifier::PinningMode mPinningMode;
const unsigned int mMinRSABits;
const char* mHostname; // non-owning - only used for pinning checks

View File

@ -20,8 +20,10 @@ public:
NS_INLINE_DECL_THREADSAFE_REFCOUNTING(SharedCertVerifier)
SharedCertVerifier(OcspDownloadConfig odc, OcspStrictConfig osc,
OcspGetConfig ogc, PinningMode pinningMode)
: mozilla::psm::CertVerifier(odc, osc, ogc, pinningMode)
OcspGetConfig ogc, uint32_t certShortLifetimeInDays,
PinningMode pinningMode)
: mozilla::psm::CertVerifier(odc, osc, ogc, certShortLifetimeInDays,
pinningMode)
{
}
};

View File

@ -181,15 +181,17 @@ bool EnsureNSSInitialized(EnsureNSSOperator op)
}
static void
GetOCSPBehaviorFromPrefs(/*out*/ CertVerifier::OcspDownloadConfig* odc,
/*out*/ CertVerifier::OcspStrictConfig* osc,
/*out*/ CertVerifier::OcspGetConfig* ogc,
const MutexAutoLock& /*proofOfLock*/)
GetRevocationBehaviorFromPrefs(/*out*/ CertVerifier::OcspDownloadConfig* odc,
/*out*/ CertVerifier::OcspStrictConfig* osc,
/*out*/ CertVerifier::OcspGetConfig* ogc,
/*out*/ uint32_t* certShortLifetimeInDays,
const MutexAutoLock& /*proofOfLock*/)
{
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(odc);
MOZ_ASSERT(osc);
MOZ_ASSERT(ogc);
MOZ_ASSERT(certShortLifetimeInDays);
// 0 = disabled, otherwise enabled
*odc = Preferences::GetInt("security.OCSP.enabled", 1)
@ -205,6 +207,13 @@ GetOCSPBehaviorFromPrefs(/*out*/ CertVerifier::OcspDownloadConfig* odc,
? CertVerifier::ocspGetEnabled
: CertVerifier::ocspGetDisabled;
// If we pass in just 0 as the second argument to Preferences::GetUint, there
// are two function signatures that match (given that 0 can be intepreted as
// a null pointer). Thus the compiler will complain without the cast.
*certShortLifetimeInDays =
Preferences::GetUint("security.pki.cert_short_lifetime_in_days",
static_cast<uint32_t>(0));
SSL_ClearSessionCache();
}
@ -875,9 +884,13 @@ void nsNSSComponent::setValidationOptions(bool isInitialSetting,
CertVerifier::OcspDownloadConfig odc;
CertVerifier::OcspStrictConfig osc;
CertVerifier::OcspGetConfig ogc;
uint32_t certShortLifetimeInDays;
GetOCSPBehaviorFromPrefs(&odc, &osc, &ogc, lock);
mDefaultCertVerifier = new SharedCertVerifier(odc, osc, ogc, pinningMode);
GetRevocationBehaviorFromPrefs(&odc, &osc, &ogc, &certShortLifetimeInDays,
lock);
mDefaultCertVerifier = new SharedCertVerifier(odc, osc, ogc,
certShortLifetimeInDays,
pinningMode);
}
// Enable the TLS versions given in the prefs, defaulting to TLS 1.0 (min) and
@ -1344,6 +1357,7 @@ nsNSSComponent::Observe(nsISupports* aSubject, const char* aTopic,
} else if (prefName.EqualsLiteral("security.OCSP.enabled") ||
prefName.EqualsLiteral("security.OCSP.require") ||
prefName.EqualsLiteral("security.OCSP.GET.enabled") ||
prefName.EqualsLiteral("security.pki.cert_short_lifetime_in_days") ||
prefName.EqualsLiteral("security.ssl.enable_ocsp_stapling") ||
prefName.EqualsLiteral("security.cert_pinning.enforcement_level")) {
MutexAutoLock lock(mutex);

View File

@ -47,6 +47,39 @@ function run_test() {
}
function add_tests() {
// Test that verifying a certificate with a "short lifetime" doesn't result
// in OCSP fetching. Due to longevity requirements in our testing
// infrastructure, the certificate we encounter is valid for a very long
// time, so we have to define a "short lifetime" as something very long.
add_test(function() {
Services.prefs.setIntPref("security.pki.cert_short_lifetime_in_days",
12000);
run_next_test();
});
add_connection_test("ocsp-stapling-none.example.com", PRErrorCodeSuccess,
clearSessionCache);
add_test(function() {
Assert.equal(0, gFetchCount,
"expected zero OCSP requests for a short-lived certificate");
Services.prefs.setIntPref("security.pki.cert_short_lifetime_in_days", 100);
run_next_test();
});
// If a "short lifetime" is something more reasonable, ensure that we do OCSP
// fetching for this long-lived certificate.
add_connection_test("ocsp-stapling-none.example.com", PRErrorCodeSuccess,
clearSessionCache);
add_test(function() {
Assert.equal(1, gFetchCount,
"expected one OCSP request for a long-lived certificate");
Services.prefs.clearUserPref("security.pki.cert_short_lifetime_in_days");
run_next_test();
});
//---------------------------------------------------------------------------
// Reset state
add_test(function() { clearOCSPCache(); gFetchCount = 0; run_next_test(); });
// This test assumes that OCSPStaplingServer uses the same cert for
// ocsp-stapling-unknown.example.com and ocsp-stapling-none.example.com.

View File

@ -107,6 +107,7 @@ private:
{
}
friend Time TimeFromElapsedSecondsAD(uint64_t);
friend class Duration;
uint64_t elapsedSecondsAD;
};
@ -121,6 +122,30 @@ Time Now();
// Note the epoch is the unix epoch (ie 00:00:00 UTC, 1 January 1970)
Time TimeFromEpochInSeconds(uint64_t secondsSinceEpoch);
class Duration final
{
public:
Duration(Time timeA, Time timeB)
: durationInSeconds(timeA < timeB
? timeB.elapsedSecondsAD - timeA.elapsedSecondsAD
: timeA.elapsedSecondsAD - timeB.elapsedSecondsAD)
{
}
explicit Duration(uint64_t durationInSeconds)
: durationInSeconds(durationInSeconds)
{
}
bool operator<(const Duration& other) const
{
return durationInSeconds < other.durationInSeconds;
}
private:
uint64_t durationInSeconds;
};
} } // namespace mozilla::pkix
#endif // mozilla_pkix_Time_h

View File

@ -268,6 +268,7 @@ public:
virtual Result CheckRevocation(EndEntityOrCA endEntityOrCA,
const CertID& certID, Time time,
Duration validityDuration,
/*optional*/ const Input* stapledOCSPresponse,
/*optional*/ const Input* aiaExtension) = 0;

View File

@ -224,8 +224,17 @@ PathBuildingStep::Check(Input potentialIssuerDER,
if (deferredSubjectError != Result::ERROR_EXPIRED_CERTIFICATE) {
CertID certID(subject.GetIssuer(), potentialIssuer.GetSubjectPublicKeyInfo(),
subject.GetSerialNumber());
Time notBefore(Time::uninitialized);
Time notAfter(Time::uninitialized);
// This should never fail. If we're here, we've already checked that the
// given time is in the certificate's validity period.
rv = CheckValidity(subject.GetValidity(), time, &notBefore, &notAfter);
if (rv != Success) {
return rv;
}
Duration validityDuration(notAfter, notBefore);
rv = trustDomain.CheckRevocation(subject.endEntityOrCA, certID, time,
stapledOCSPResponse,
validityDuration, stapledOCSPResponse,
subject.GetAuthorityInfoAccess());
if (rv != Success) {
return RecordResult(rv, keepGoing);

View File

@ -125,7 +125,9 @@ CheckSignatureAlgorithm(TrustDomain& trustDomain,
// 4.1.2.5 Validity
Result
CheckValidity(Input encodedValidity, Time time)
CheckValidity(Input encodedValidity, Time time,
/*optional out*/ Time* notBeforeOut,
/*optional out*/ Time* notAfterOut)
{
Reader validity(encodedValidity);
Time notBefore(Time::uninitialized);
@ -154,6 +156,12 @@ CheckValidity(Input encodedValidity, Time time)
return Result::ERROR_EXPIRED_CERTIFICATE;
}
if (notBeforeOut) {
*notBeforeOut = notBefore;
}
if (notAfterOut) {
*notAfterOut = notAfter;
}
return Success;
}

View File

@ -45,6 +45,10 @@ Result CheckNameConstraints(Input encodedNameConstraints,
const BackCert& firstChild,
KeyPurposeId requiredEKUIfPresent);
Result CheckValidity(Input encodedValidity, Time time,
/*optional out*/ Time* notBeforeOut = nullptr,
/*optional out*/ Time* notAfterOut = nullptr);
} } // namespace mozilla::pkix
#endif // mozilla_pkix_pkixcheck_h

View File

@ -139,7 +139,7 @@ private:
return Success;
}
Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*, /*optional*/ const Input*)
override
{
@ -403,7 +403,7 @@ public:
return Success;
}
Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*, /*optional*/ const Input*)
override
{

View File

@ -68,7 +68,7 @@ private:
return Success;
}
Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*, /*optional*/ const Input*)
override
{

View File

@ -91,8 +91,8 @@ private:
return checker.Check(issuerCert, nullptr, keepGoing);
}
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, const Input*,
const Input*) override
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
const Input*, const Input*) override
{
return Success;
}

View File

@ -292,7 +292,7 @@ public:
return Success;
}
Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*,
/*optional*/ const Input*) override
{

View File

@ -22,17 +22,12 @@
* limitations under the License.
*/
#include "pkixcheck.h"
#include "pkixgtest.h"
using namespace mozilla::pkix;
using namespace mozilla::pkix::test;
namespace mozilla { namespace pkix {
Result CheckValidity(const Input encodedValidity, Time time);
} } // namespace mozilla::pkix
static const Time PAST_TIME(YMDHMS(1998, 12, 31, 12, 23, 56));
#define OLDER_GENERALIZEDTIME \

View File

@ -102,7 +102,7 @@ public:
Result::FATAL_ERROR_LIBRARY_FAILURE);
}
Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration,
/*optional*/ const Input*,
/*optional*/ const Input*) override
{