diff --git a/security/certverifier/CertVerifier.cpp b/security/certverifier/CertVerifier.cpp index 9e15562d9fd4..d59a664b49c9 100644 --- a/security/certverifier/CertVerifier.cpp +++ b/security/certverifier/CertVerifier.cpp @@ -272,7 +272,8 @@ CertVerifier::InsanityVerifyCert( case certificateUsageSSLClient: { // 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, pinArg); + NSSCertDBTrustDomain trustDomain(trustEmail, ocspFetching, mOCSPCache, + pinArg); rv = BuildCertChain(trustDomain, cert, time, MustBeEndEntity, KU_DIGITAL_SIGNATURE, SEC_OID_EXT_KEY_USAGE_CLIENT_AUTH, @@ -296,7 +297,7 @@ CertVerifier::InsanityVerifyCert( ocspFetching == NSSCertDBTrustDomain::NeverFetchOCSP ? NSSCertDBTrustDomain::LocalOnlyOCSPForEV : NSSCertDBTrustDomain::FetchOCSPForEV, - pinArg); + mOCSPCache, pinArg); rv = BuildCertChainForOneKeyUsage(trustDomain, cert, time, KU_DIGITAL_SIGNATURE, // ECDHE/DHE KU_KEY_ENCIPHERMENT, // RSA @@ -321,7 +322,8 @@ CertVerifier::InsanityVerifyCert( } // Now try non-EV. - NSSCertDBTrustDomain trustDomain(trustSSL, ocspFetching, pinArg); + NSSCertDBTrustDomain trustDomain(trustSSL, ocspFetching, mOCSPCache, + pinArg); rv = BuildCertChainForOneKeyUsage(trustDomain, cert, time, KU_DIGITAL_SIGNATURE, // ECDHE/DHE KU_KEY_ENCIPHERMENT, // RSA @@ -333,7 +335,8 @@ CertVerifier::InsanityVerifyCert( } case certificateUsageSSLCA: { - NSSCertDBTrustDomain trustDomain(trustSSL, ocspFetching, pinArg); + NSSCertDBTrustDomain trustDomain(trustSSL, ocspFetching, mOCSPCache, + pinArg); rv = BuildCertChain(trustDomain, cert, time, MustBeCA, KU_KEY_CERT_SIGN, SEC_OID_EXT_KEY_USAGE_SERVER_AUTH, @@ -343,7 +346,8 @@ CertVerifier::InsanityVerifyCert( } case certificateUsageEmailSigner: { - NSSCertDBTrustDomain trustDomain(trustEmail, ocspFetching, pinArg); + NSSCertDBTrustDomain trustDomain(trustEmail, ocspFetching, mOCSPCache, + pinArg); rv = BuildCertChain(trustDomain, cert, time, MustBeEndEntity, KU_DIGITAL_SIGNATURE, SEC_OID_EXT_KEY_USAGE_EMAIL_PROTECT, @@ -356,7 +360,8 @@ CertVerifier::InsanityVerifyCert( // TODO: The higher level S/MIME processing should pass in which key // usage it is trying to verify for, and base its algorithm choices // based on the result of the verification(s). - NSSCertDBTrustDomain trustDomain(trustEmail, ocspFetching, pinArg); + NSSCertDBTrustDomain trustDomain(trustEmail, ocspFetching, mOCSPCache, + pinArg); rv = BuildCertChainForOneKeyUsage(trustDomain, cert, time, KU_KEY_ENCIPHERMENT, // RSA KU_KEY_AGREEMENT, // ECDH/DH @@ -369,7 +374,7 @@ CertVerifier::InsanityVerifyCert( case certificateUsageObjectSigner: { NSSCertDBTrustDomain trustDomain(trustObjectSigning, ocspFetching, - pinArg); + mOCSPCache, pinArg); rv = BuildCertChain(trustDomain, cert, time, MustBeEndEntity, KU_DIGITAL_SIGNATURE, SEC_OID_EXT_KEY_USAGE_CODE_SIGN, @@ -397,18 +402,21 @@ CertVerifier::InsanityVerifyCert( eku = SEC_OID_OCSP_RESPONDER; } - NSSCertDBTrustDomain sslTrust(trustSSL, ocspFetching, pinArg); + NSSCertDBTrustDomain sslTrust(trustSSL, ocspFetching, mOCSPCache, + pinArg); rv = BuildCertChain(sslTrust, cert, time, endEntityOrCA, keyUsage, eku, SEC_OID_X509_ANY_POLICY, stapledOCSPResponse, builtChain); if (rv == SECFailure && PR_GetError() == SEC_ERROR_UNKNOWN_ISSUER) { - NSSCertDBTrustDomain emailTrust(trustEmail, ocspFetching, pinArg); + NSSCertDBTrustDomain emailTrust(trustEmail, ocspFetching, mOCSPCache, + pinArg); rv = BuildCertChain(emailTrust, cert, time, endEntityOrCA, keyUsage, eku, SEC_OID_X509_ANY_POLICY, stapledOCSPResponse, builtChain); if (rv == SECFailure && SEC_ERROR_UNKNOWN_ISSUER) { NSSCertDBTrustDomain objectSigningTrust(trustObjectSigning, - ocspFetching, pinArg); + ocspFetching, mOCSPCache, + pinArg); rv = BuildCertChain(objectSigningTrust, cert, time, endEntityOrCA, keyUsage, eku, SEC_OID_X509_ANY_POLICY, stapledOCSPResponse, builtChain); diff --git a/security/certverifier/CertVerifier.h b/security/certverifier/CertVerifier.h index 66146c6467c0..2c970001e60a 100644 --- a/security/certverifier/CertVerifier.h +++ b/security/certverifier/CertVerifier.h @@ -8,6 +8,7 @@ #define mozilla_psm__CertVerifier_h #include "insanity/pkixtypes.h" +#include "OCSPCache.h" namespace mozilla { namespace psm { @@ -67,6 +68,8 @@ public: ocsp_get_config ogc); ~CertVerifier(); + void ClearOCSPCache() { mOCSPCache.Clear(); } + const implementation_config mImplementation; #ifndef NSS_NO_LIBPKIX const bool mMissingCertDownloadEnabled; @@ -85,6 +88,8 @@ private: /*optional*/ const SECItem* stapledOCSPResponse, /*optional out*/ insanity::pkix::ScopedCERTCertList* validationChain, /*optional out*/ SECOidTag* evOidPolicy); + + OCSPCache mOCSPCache; }; void InitCertVerifierLog(); diff --git a/security/certverifier/NSSCertDBTrustDomain.cpp b/security/certverifier/NSSCertDBTrustDomain.cpp index 661699f610e7..b047ca241ee2 100644 --- a/security/certverifier/NSSCertDBTrustDomain.cpp +++ b/security/certverifier/NSSCertDBTrustDomain.cpp @@ -9,8 +9,8 @@ #include #include "ExtendedValidation.h" -#include "insanity/pkix.h" #include "certdb.h" +#include "insanity/pkix.h" #include "nss.h" #include "ocsp.h" #include "pk11pub.h" @@ -30,9 +30,9 @@ namespace mozilla { namespace psm { const char BUILTIN_ROOTS_MODULE_DEFAULT_NAME[] = "Builtin Roots Module"; -namespace { +void PORT_Free_string(char* str) { PORT_Free(str); } -inline void PORT_Free_string(char* str) { PORT_Free(str); } +namespace { typedef ScopedPtr ScopedSECMODModule; @@ -40,9 +40,11 @@ typedef ScopedPtr ScopedSECMODModule; NSSCertDBTrustDomain::NSSCertDBTrustDomain(SECTrustType certDBTrustType, OCSPFetching ocspFetching, + OCSPCache& ocspCache, void* pinArg) : mCertDBTrustType(certDBTrustType) , mOCSPFetching(ocspFetching) + , mOCSPCache(ocspCache) , mPinArg(pinArg) { } @@ -161,17 +163,62 @@ NSSCertDBTrustDomain::CheckRevocation( // are known to serve expired responses due to bugs. if (stapledOCSPResponse) { PR_ASSERT(endEntityOrCA == MustBeEndEntity); - SECStatus rv = VerifyEncodedOCSPResponse(*this, cert, issuerCert, time, - stapledOCSPResponse); + SECStatus rv = VerifyAndMaybeCacheEncodedOCSPResponse(cert, issuerCert, + time, + stapledOCSPResponse); if (rv == SECSuccess) { + PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, + ("NSSCertDBTrustDomain: stapled OCSP response: good")); return rv; } if (PR_GetError() != SEC_ERROR_OCSP_OLD_RESPONSE) { + PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, + ("NSSCertDBTrustDomain: stapled OCSP response: failure")); return rv; } + } else { + PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, + ("NSSCertDBTrustDomain: no stapled OCSP response")); } - // TODO(bug 915932): Need to change this when we add the OCSP cache. + PRErrorCode cachedResponseErrorCode = 0; + PRTime cachedResponseValidThrough = 0; + bool cachedResponsePresent = mOCSPCache.Get(cert, issuerCert, + cachedResponseErrorCode, + cachedResponseValidThrough); + if (cachedResponsePresent) { + if (cachedResponseErrorCode == 0 && cachedResponseValidThrough >= time) { + PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, + ("NSSCertDBTrustDomain: cached OCSP response: good")); + return SECSuccess; + } + // If we have a cached revoked response, use it. + if (cachedResponseErrorCode == SEC_ERROR_REVOKED_CERTIFICATE) { + PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, + ("NSSCertDBTrustDomain: cached OCSP response: revoked")); + PR_SetError(SEC_ERROR_REVOKED_CERTIFICATE, 0); + return SECFailure; + } + // The cached response may indicate an unknown certificate or it may be + // expired. Don't return with either of these statuses yet - we may be + // able to fetch a more recent one. + PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, + ("NSSCertDBTrustDomain: cached OCSP response: error %ld valid " + "until %lld", cachedResponseErrorCode, cachedResponseValidThrough)); + // When a good cached response has expired, it is more convenient + // to convert that to an error code and just deal with + // cachedResponseErrorCode from here on out. + if (cachedResponseErrorCode == 0 && cachedResponseValidThrough < time) { + cachedResponseErrorCode = SEC_ERROR_OCSP_OLD_RESPONSE; + } + } else { + PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, + ("NSSCertDBTrustDomain: no cached OCSP response")); + } + // At this point, if and only if cachedErrorResponseCode is 0, there was no + // cached response. + PR_ASSERT((!cachedResponsePresent && cachedResponseErrorCode == 0) || + (cachedResponsePresent && cachedResponseErrorCode != 0)); // TODO: We still need to handle the fallback for expired responses. But, // if/when we disable OCSP fetching by default, it would be ambiguous whether @@ -181,11 +228,26 @@ NSSCertDBTrustDomain::CheckRevocation( if ((mOCSPFetching == NeverFetchOCSP) || (endEntityOrCA == MustBeCA && (mOCSPFetching == FetchOCSPForDVHardFail || mOCSPFetching == FetchOCSPForDVSoftFail))) { + // We're not going to be doing any fetching, so if there was a cached + // "unknown" response, say so. + if (cachedResponseErrorCode == SEC_ERROR_OCSP_UNKNOWN_CERT) { + PR_SetError(SEC_ERROR_OCSP_UNKNOWN_CERT, 0); + return SECFailure; + } + // If we're doing hard-fail, we want to know if we have a cached response + // that has expired. + if (mOCSPFetching == FetchOCSPForDVHardFail && + cachedResponseErrorCode == SEC_ERROR_OCSP_OLD_RESPONSE) { + PR_SetError(SEC_ERROR_OCSP_OLD_RESPONSE, 0); + return SECFailure; + } + return SECSuccess; } if (mOCSPFetching == LocalOnlyOCSPForEV) { - PR_SetError(SEC_ERROR_OCSP_UNKNOWN_CERT, 0); + PR_SetError(cachedResponseErrorCode != 0 ? cachedResponseErrorCode + : SEC_ERROR_OCSP_UNKNOWN_CERT, 0); return SECFailure; } @@ -193,12 +255,14 @@ NSSCertDBTrustDomain::CheckRevocation( url(CERT_GetOCSPAuthorityInfoAccessLocation(cert)); if (!url) { - if (stapledOCSPResponse) { - PR_SetError(SEC_ERROR_OCSP_OLD_RESPONSE, 0); + if (mOCSPFetching == FetchOCSPForEV || + cachedResponseErrorCode == SEC_ERROR_OCSP_UNKNOWN_CERT) { + PR_SetError(SEC_ERROR_OCSP_UNKNOWN_CERT, 0); return SECFailure; } - if (mOCSPFetching == FetchOCSPForEV) { - PR_SetError(SEC_ERROR_OCSP_UNKNOWN_CERT, 0); + if (stapledOCSPResponse || + cachedResponseErrorCode == SEC_ERROR_OCSP_OLD_RESPONSE) { + PR_SetError(SEC_ERROR_OCSP_OLD_RESPONSE, 0); return SECFailure; } @@ -229,6 +293,13 @@ NSSCertDBTrustDomain::CheckRevocation( "CERT_PostOCSPRequest failure")); return SECFailure; } + if (cachedResponseErrorCode == SEC_ERROR_OCSP_UNKNOWN_CERT) { + PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, + ("NSSCertDBTrustDomain: returning SECFailure from cached " + "response after CERT_PostOCSPRequest failure")); + PR_SetError(cachedResponseErrorCode, 0); + return SECFailure; + } PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, ("NSSCertDBTrustDomain: returning SECSuccess after " @@ -236,8 +307,8 @@ NSSCertDBTrustDomain::CheckRevocation( return SECSuccess; // Soft fail -> success :( } - SECStatus rv = VerifyEncodedOCSPResponse(*this, cert, issuerCert, time, - response); + SECStatus rv = VerifyAndMaybeCacheEncodedOCSPResponse(cert, issuerCert, time, + response); if (rv == SECSuccess || mOCSPFetching != FetchOCSPForDVSoftFail) { PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, ("NSSCertDBTrustDomain: returning after VerifyEncodedOCSPResponse")); @@ -256,6 +327,31 @@ NSSCertDBTrustDomain::CheckRevocation( return SECSuccess; } +SECStatus +NSSCertDBTrustDomain::VerifyAndMaybeCacheEncodedOCSPResponse( + const CERTCertificate* cert, CERTCertificate* issuerCert, PRTime time, + const SECItem* encodedResponse) +{ + PRTime thisUpdate = 0; + PRTime validThrough = 0; + SECStatus rv = VerifyEncodedOCSPResponse(*this, cert, issuerCert, time, + encodedResponse, &thisUpdate, + &validThrough); + PRErrorCode error = (rv == SECSuccess ? 0 : PR_GetError()); + if (rv == SECSuccess || error == SEC_ERROR_REVOKED_CERTIFICATE || + error == SEC_ERROR_OCSP_UNKNOWN_CERT) { + PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, + ("NSSCertDBTrustDomain: caching OCSP response")); + if (mOCSPCache.Put(cert, issuerCert, error, thisUpdate, validThrough) + != SECSuccess) { + return SECFailure; + } + // The call to Put may have un-set the error. Re-set it. + PR_SetError(error, 0); + } + return rv; +} + namespace { static char* diff --git a/security/certverifier/NSSCertDBTrustDomain.h b/security/certverifier/NSSCertDBTrustDomain.h index 730d0998e752..3a5c0ca79582 100644 --- a/security/certverifier/NSSCertDBTrustDomain.h +++ b/security/certverifier/NSSCertDBTrustDomain.h @@ -19,6 +19,8 @@ void DisableMD5(); extern const char BUILTIN_ROOTS_MODULE_DEFAULT_NAME[]; +void PORT_Free_string(char* str); + // The dir parameter is the path to the directory containing the NSS builtin // roots module. Usually this is the same as the path to the other NSS shared // libraries. If it is null then the (library) path will be searched. @@ -55,7 +57,7 @@ public: LocalOnlyOCSPForEV = 4, }; NSSCertDBTrustDomain(SECTrustType certDBTrustType, OCSPFetching ocspFetching, - void* pinArg); + OCSPCache& ocspCache, void* pinArg); virtual SECStatus FindPotentialIssuers( const SECItem* encodedIssuerName, @@ -77,8 +79,13 @@ public: /*optional*/ const SECItem* stapledOCSPResponse); private: + SECStatus VerifyAndMaybeCacheEncodedOCSPResponse( + const CERTCertificate* cert, CERTCertificate* issuerCert, PRTime time, + const SECItem* encodedResponse); + const SECTrustType mCertDBTrustType; const OCSPFetching mOCSPFetching; + OCSPCache& mOCSPCache; // non-owning! void* mPinArg; // non-owning! }; diff --git a/security/certverifier/OCSPCache.cpp b/security/certverifier/OCSPCache.cpp new file mode 100644 index 000000000000..f37794d2d9a0 --- /dev/null +++ b/security/certverifier/OCSPCache.cpp @@ -0,0 +1,289 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* Copyright 2013 Mozilla Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "OCSPCache.h" + +#include "NSSCertDBTrustDomain.h" +#include "pk11pub.h" +#include "secerr.h" + +#ifdef PR_LOGGING +extern PRLogModuleInfo* gCertVerifierLog; +#endif + +namespace mozilla { namespace psm { + +void +Insanity_PK11_DestroyContext_true(PK11Context* context) +{ + PK11_DestroyContext(context, true); +} + +typedef insanity::pkix::ScopedPtr + ScopedPK11Context; + +// Let derIssuer be the DER encoding of the issuer of aCert. +// Let derPublicKey be the DER encoding of the public key of aIssuerCert. +// Let serialNumber be the bytes of the serial number of aCert. +// The value calculated is SHA384(derIssuer || derPublicKey || serialNumber). +// Because the DER encodings include the length of the data encoded, +// there do not exist A(derIssuerA, derPublicKeyA, serialNumberA) and +// B(derIssuerB, derPublicKeyB, serialNumberB) such that the concatenation of +// each triplet results in the same string of bytes but where each part in A is +// not equal to its counterpart in B. This is important because as a result it +// is computationally infeasible to find collisions that would subvert this +// cache (given that SHA384 is a cryptographically-secure hash function). +static SECStatus +CertIDHash(SHA384Buffer& buf, const CERTCertificate* aCert, + const CERTCertificate* aIssuerCert) +{ + ScopedPK11Context context(PK11_CreateDigestContext(SEC_OID_SHA384)); + if (!context) { + return SECFailure; + } + SECStatus rv = PK11_DigestBegin(context.get()); + if (rv != SECSuccess) { + return rv; + } + rv = PK11_DigestOp(context.get(), aCert->derIssuer.data, + aCert->derIssuer.len); + if (rv != SECSuccess) { + return rv; + } + rv = PK11_DigestOp(context.get(), aIssuerCert->derPublicKey.data, + aIssuerCert->derPublicKey.len); + if (rv != SECSuccess) { + return rv; + } + rv = PK11_DigestOp(context.get(), aCert->serialNumber.data, + aCert->serialNumber.len); + if (rv != SECSuccess) { + return rv; + } + uint32_t outLen = 0; + rv = PK11_DigestFinal(context.get(), buf, &outLen, SHA384_LENGTH); + if (outLen != SHA384_LENGTH) { + return SECFailure; + } + return rv; +} + +SECStatus +OCSPCache::Entry::Init(const CERTCertificate* aCert, + const CERTCertificate* aIssuerCert, + PRErrorCode aErrorCode, + PRTime aThisUpdate, + PRTime aValidThrough) +{ + mErrorCode = aErrorCode; + mThisUpdate = aThisUpdate; + mValidThrough = aValidThrough; + return CertIDHash(mIDHash, aCert, aIssuerCert); +} + +OCSPCache::OCSPCache() + : mMutex("OCSPCache-mutex") +{ +} + +OCSPCache::~OCSPCache() +{ + Clear(); +} + +// Returns -1 if no entry is found for the given (cert, issuer) pair. +int32_t +OCSPCache::FindInternal(const CERTCertificate* aCert, + const CERTCertificate* aIssuerCert, + const MutexAutoLock& /* aProofOfLock */) +{ + if (mEntries.length() == 0) { + return -1; + } + + SHA384Buffer idHash; + SECStatus rv = CertIDHash(idHash, aCert, aIssuerCert); + if (rv != SECSuccess) { + return -1; + } + + // mEntries is sorted with the most-recently-used entry at the end. + // Thus, searching from the end will often be fastest. + for (int32_t i = mEntries.length() - 1; i >= 0; i--) { + if (memcmp(mEntries[i]->mIDHash, idHash, SHA384_LENGTH) == 0) { + return i; + } + } + return -1; +} + +void +OCSPCache::LogWithCerts(const char* aMessage, const CERTCertificate* aCert, + const CERTCertificate* aIssuerCert) +{ +#ifdef PR_LOGGING + if (PR_LOG_TEST(gCertVerifierLog, PR_LOG_DEBUG)) { + insanity::pkix::ScopedPtr + cn(CERT_GetCommonName(&aCert->subject)); + insanity::pkix::ScopedPtr + cnIssuer(CERT_GetCommonName(&aIssuerCert->subject)); + PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, (aMessage, cn.get(), cnIssuer.get())); + } +#endif +} + +void +OCSPCache::MakeMostRecentlyUsed(size_t aIndex, + const MutexAutoLock& /* aProofOfLock */) +{ + Entry* entry = mEntries[aIndex]; + // Since mEntries is sorted with the most-recently-used entry at the end, + // aIndex is likely to be near the end, so this is likely to be fast. + mEntries.erase(mEntries.begin() + aIndex); + mEntries.append(entry); +} + +bool +OCSPCache::Get(const CERTCertificate* aCert, + const CERTCertificate* aIssuerCert, + PRErrorCode& aErrorCode, + PRTime& aValidThrough) +{ + PR_ASSERT(aCert); + PR_ASSERT(aIssuerCert); + + MutexAutoLock lock(mMutex); + + int32_t index = FindInternal(aCert, aIssuerCert, lock); + if (index < 0) { + LogWithCerts("OCSPCache::Get(%s, %s) not in cache", aCert, aIssuerCert); + return false; + } + LogWithCerts("OCSPCache::Get(%s, %s) in cache", aCert, aIssuerCert); + aErrorCode = mEntries[index]->mErrorCode; + aValidThrough = mEntries[index]->mValidThrough; + MakeMostRecentlyUsed(index, lock); + return true; +} + +SECStatus +OCSPCache::Put(const CERTCertificate* aCert, + const CERTCertificate* aIssuerCert, + PRErrorCode aErrorCode, + PRTime aThisUpdate, + PRTime aValidThrough) +{ + PR_ASSERT(aCert); + PR_ASSERT(aIssuerCert); + + MutexAutoLock lock(mMutex); + + int32_t index = FindInternal(aCert, aIssuerCert, lock); + + if (index >= 0) { + // Never replace an entry indicating a revoked certificate. + if (mEntries[index]->mErrorCode == SEC_ERROR_REVOKED_CERTIFICATE) { + LogWithCerts("OCSPCache::Put(%s, %s) already in cache as revoked - " + "not replacing", aCert, aIssuerCert); + MakeMostRecentlyUsed(index, lock); + return SECSuccess; + } + + // Never replace a newer entry with an older one unless the older entry + // indicates a revoked certificate, which we want to remember. + if (mEntries[index]->mThisUpdate > aThisUpdate && + aErrorCode != SEC_ERROR_REVOKED_CERTIFICATE) { + LogWithCerts("OCSPCache::Put(%s, %s) already in cache with more recent " + "validity - not replacing", aCert, aIssuerCert); + MakeMostRecentlyUsed(index, lock); + return SECSuccess; + } + + LogWithCerts("OCSPCache::Put(%s, %s) already in cache - replacing", + aCert, aIssuerCert); + mEntries[index]->mErrorCode = aErrorCode; + mEntries[index]->mThisUpdate = aThisUpdate; + mEntries[index]->mValidThrough = aValidThrough; + MakeMostRecentlyUsed(index, lock); + return SECSuccess; + } + + if (mEntries.length() == MaxEntries) { + LogWithCerts("OCSPCache::Put(%s, %s) too full - evicting an entry", aCert, + aIssuerCert); + for (Entry** toEvict = mEntries.begin(); toEvict != mEntries.end(); + toEvict++) { + // Never evict an entry that indicates a revoked or unknokwn certificate, + // because revoked responses are more security-critical to remember. + if ((*toEvict)->mErrorCode != SEC_ERROR_REVOKED_CERTIFICATE && + (*toEvict)->mErrorCode != SEC_ERROR_OCSP_UNKNOWN_CERT) { + delete *toEvict; + mEntries.erase(toEvict); + break; + } + } + // Well, we tried, but apparently everything is revoked or unknown. + // We don't want to remove a cached revoked or unknown response. If we're + // trying to insert a good response, we can just return "successfully" + // without doing so. This means we'll lose some speed, but it's not a + // security issue. If we're trying to insert a revoked or unknown response, + // we can't. We should return with an error that causes the current + // verification to fail. + if (mEntries.length() == MaxEntries) { + if (aErrorCode != 0) { + PR_SetError(aErrorCode, 0); + return SECFailure; + } + return SECSuccess; + } + } + + Entry* newEntry = new Entry(); + // Normally we don't have to do this in Gecko, because OOM is fatal. + // However, if we want to embed this in another project, OOM might not + // be fatal, so handle this case. + if (!newEntry) { + PR_SetError(SEC_ERROR_NO_MEMORY, 0); + return SECFailure; + } + SECStatus rv = newEntry->Init(aCert, aIssuerCert, aErrorCode, aThisUpdate, + aValidThrough); + if (rv != SECSuccess) { + return rv; + } + mEntries.append(newEntry); + LogWithCerts("OCSPCache::Put(%s, %s) added to cache", aCert, aIssuerCert); + return SECSuccess; +} + +void +OCSPCache::Clear() +{ + MutexAutoLock lock(mMutex); + PR_LOG(gCertVerifierLog, PR_LOG_DEBUG, ("OCSPCache::Clear: clearing cache")); + // First go through and delete the memory being pointed to by the pointers + // in the vector. + for (Entry** entry = mEntries.begin(); entry < mEntries.end(); + entry++) { + delete *entry; + } + // Then remove the pointers themselves. + mEntries.clearAndFree(); +} + +} } // namespace mozilla::psm diff --git a/security/certverifier/OCSPCache.h b/security/certverifier/OCSPCache.h new file mode 100644 index 000000000000..61db84c64a72 --- /dev/null +++ b/security/certverifier/OCSPCache.h @@ -0,0 +1,107 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* Copyright 2013 Mozilla Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef mozilla_psm_OCSPCache_h +#define mozilla_psm_OCSPCache_h + +#include "certt.h" +#include "hasht.h" +#include "insanity/pkixtypes.h" +#include "mozilla/Mutex.h" +#include "mozilla/Vector.h" +#include "prerror.h" + +namespace mozilla { namespace psm { + +// make SHA384Buffer be of type "array of uint8_t of length SHA384_LENGTH" +typedef uint8_t SHA384Buffer[SHA384_LENGTH]; + +// OCSPCache can store and retrieve OCSP response verification results. Each +// result is keyed on the certificate that purportedly corresponds to it (where +// certificates are distinguished based on serial number, issuer, and +// issuer public key, much like in an encoded OCSP response itself). A maximum +// of 1024 distinct entries can be stored. +// OCSPCache is thread-safe. +class OCSPCache +{ +public: + OCSPCache(); + ~OCSPCache(); + + // Returns true if the status of the given certificate (issued by the given + // issuer) is in the cache, and false otherwise. + // If it is in the cache, returns by reference the error code of the cached + // status and the time through which the status is considered trustworthy. + bool Get(const CERTCertificate* aCert, const CERTCertificate* aIssuerCert, + /* out */ PRErrorCode& aErrorCode, /* out */ PRTime& aValidThrough); + + // Caches the status of the given certificate (issued by the given issuer). + // The status is considered trustworthy through the given time. + // A status with an error code of SEC_ERROR_REVOKED_CERTIFICATE will not + // be replaced or evicted. + // A status with an error code of SEC_ERROR_OCSP_UNKNOWN_CERT will not + // be evicted when the cache is full. + // A status with a more recent thisUpdate will not be replaced with a + // status with a less recent thisUpdate unless the less recent status + // indicates the certificate is revoked. + SECStatus Put(const CERTCertificate* aCert, + const CERTCertificate* aIssuerCert, + PRErrorCode aErrorCode, + PRTime aThisUpdate, + PRTime aValidThrough); + + // Removes everything from the cache. + void Clear(); + +private: + class Entry + { + public: + SECStatus Init(const CERTCertificate* aCert, + const CERTCertificate* aIssuerCert, + PRErrorCode aErrorCode, PRTime aThisUpdate, + PRTime aValidThrough); + + PRErrorCode mErrorCode; + PRTime mThisUpdate; + PRTime mValidThrough; + // The SHA-384 hash of the concatenation of the DER encodings of the + // issuer name and issuer key, followed by the serial number. + // See the documentation for CertIDHash in OCSPCache.cpp. + SHA384Buffer mIDHash; + }; + + int32_t FindInternal(const CERTCertificate* aCert, + const CERTCertificate* aIssuerCert, + const MutexAutoLock& aProofOfLock); + void MakeMostRecentlyUsed(size_t aIndex, const MutexAutoLock& aProofOfLock); + void LogWithCerts(const char* aMessage, const CERTCertificate* aCert, + const CERTCertificate* aIssuerCert); + + Mutex mMutex; + static const size_t MaxEntries = 1024; + // Sorted with the most-recently-used entry at the end. + // Using 256 here reserves as much possible inline storage as the vector + // implementation will give us. 1024 bytes is the maximum it allows, + // which results in 256 Entry pointers or 128 Entry pointers, depending + // on the size of a pointer. + Vector mEntries; +}; + +} } // namespace mozilla::psm + +#endif // mozilla_psm_OCSPCache_h diff --git a/security/certverifier/moz.build b/security/certverifier/moz.build index 4855731fbf81..dac05e5c3481 100644 --- a/security/certverifier/moz.build +++ b/security/certverifier/moz.build @@ -7,6 +7,7 @@ UNIFIED_SOURCES += [ 'CertVerifier.cpp', 'NSSCertDBTrustDomain.cpp', + 'OCSPCache.cpp', ] if not CONFIG['NSS_NO_EV_CERTS']: diff --git a/security/insanity/include/insanity/pkix.h b/security/insanity/include/insanity/pkix.h index d0d2598a0850..32eb2d925859 100644 --- a/security/insanity/include/insanity/pkix.h +++ b/security/insanity/include/insanity/pkix.h @@ -103,11 +103,20 @@ SECItem* CreateEncodedOCSPRequest(PLArenaPool* arena, const CERTCertificate* cert, const CERTCertificate* issuerCert); +// The optional parameter thisUpdate will be the thisUpdate value of +// the encoded response if it is considered trustworthy. Only +// good, unknown, or revoked responses that verify correctly are considered +// trustworthy. If the response is not trustworthy, thisUpdate will be 0. +// Similarly, the optional parameter validThrough will be the time through +// which the encoded response is considered trustworthy (that is, if a response had a +// thisUpdate time of validThrough, it would be considered trustworthy). SECStatus VerifyEncodedOCSPResponse(TrustDomain& trustDomain, const CERTCertificate* cert, CERTCertificate* issuerCert, PRTime time, - const SECItem* encodedResponse); + const SECItem* encodedResponse, + /* optional out */ PRTime* thisUpdate, + /* optional out */ PRTime* validThrough); } } // namespace insanity::pkix diff --git a/security/insanity/lib/pkixocsp.cpp b/security/insanity/lib/pkixocsp.cpp index 06e7efd09c6a..c9e3056178ee 100644 --- a/security/insanity/lib/pkixocsp.cpp +++ b/security/insanity/lib/pkixocsp.cpp @@ -55,13 +55,23 @@ public: Context(TrustDomain& trustDomain, const CERTCertificate& cert, CERTCertificate& issuerCert, - PRTime time) + PRTime time, + PRTime* thisUpdate, + PRTime* validThrough) : trustDomain(trustDomain) , cert(cert) , issuerCert(issuerCert) , time(time) , certStatus(CertStatus::Unknown) + , thisUpdate(thisUpdate) + , validThrough(validThrough) { + if (thisUpdate) { + *thisUpdate = 0; + } + if (validThrough) { + *validThrough = 0; + } } TrustDomain& trustDomain; @@ -69,6 +79,8 @@ public: CERTCertificate& issuerCert; const PRTime time; CertStatus certStatus; + PRTime* thisUpdate; + PRTime* validThrough; private: Context(const Context&); // delete @@ -303,7 +315,9 @@ SECStatus VerifyEncodedOCSPResponse(TrustDomain& trustDomain, const CERTCertificate* cert, CERTCertificate* issuerCert, PRTime time, - const SECItem* encodedResponse) + const SECItem* encodedResponse, + PRTime* thisUpdate, + PRTime* validThrough) { PR_ASSERT(cert); PR_ASSERT(issuerCert); @@ -319,7 +333,8 @@ VerifyEncodedOCSPResponse(TrustDomain& trustDomain, return SECFailure; } - Context context(trustDomain, *cert, *issuerCert, time); + Context context(trustDomain, *cert, *issuerCert, time, thisUpdate, + validThrough); if (der::Nested(input, der::SEQUENCE, bind(OCSPResponse, _1, ref(context))) != der::Success) { @@ -684,6 +699,13 @@ SingleResponse(der::Input& input, Context& context) } } + if (context.thisUpdate) { + *context.thisUpdate = thisUpdate; + } + if (context.validThrough) { + *context.validThrough = notAfter; + } + return der::Success; } diff --git a/security/manager/ssl/public/nsIX509CertDB.idl b/security/manager/ssl/public/nsIX509CertDB.idl index 34fe04a3113b..1fbf2a33129f 100644 --- a/security/manager/ssl/public/nsIX509CertDB.idl +++ b/security/manager/ssl/public/nsIX509CertDB.idl @@ -21,7 +21,7 @@ interface nsIX509CertList; typedef uint32_t AppTrustedRoot; -[scriptable, function, uuid(e12aec59-7153-4e84-9376-2e851311b7a3)] +[scriptable, function, uuid(0927baea-622d-4e41-a76d-255af426e7fb)] interface nsIOpenSignedAppFileCallback : nsISupports { void openSignedAppFileFinished(in nsresult rv, @@ -358,4 +358,7 @@ interface nsIX509CertDB : nsISupports { out nsIX509CertList verifiedChain, out bool aHasEVPolicy); + // Clears the OCSP cache for the current certificate verification + // implementation. + void clearOCSPCache(); }; diff --git a/security/manager/ssl/src/SSLServerCertVerification.cpp b/security/manager/ssl/src/SSLServerCertVerification.cpp index 4c4f1d3e5508..6d2ab8c0b1b0 100644 --- a/security/manager/ssl/src/SSLServerCertVerification.cpp +++ b/security/manager/ssl/src/SSLServerCertVerification.cpp @@ -909,51 +909,53 @@ AuthCertificate(CertVerifier& certVerifier, TransportSecurityInfo* infoObject, // TODO: Remove this after we switch to insanity::pkix as the // only option - if (stapledOCSPResponse) { - CERTCertDBHandle* handle = CERT_GetDefaultCertDB(); - rv = CERT_CacheOCSPResponseFromSideChannel(handle, cert, PR_Now(), - stapledOCSPResponse, - infoObject); - if (rv != SECSuccess) { - // Due to buggy servers that will staple expired OCSP responses - // (see for example http://trac.nginx.org/nginx/ticket/425), - // don't terminate the connection if the stapled response is expired. - // We will fall back to fetching revocation information. - PRErrorCode ocspErrorCode = PR_GetError(); - if (ocspErrorCode != SEC_ERROR_OCSP_OLD_RESPONSE) { - // stapled OCSP response present but invalid for some reason - Telemetry::Accumulate(Telemetry::SSL_OCSP_STAPLING, 4); - return rv; + if (certVerifier.mImplementation == CertVerifier::classic) { + if (stapledOCSPResponse) { + CERTCertDBHandle* handle = CERT_GetDefaultCertDB(); + rv = CERT_CacheOCSPResponseFromSideChannel(handle, cert, PR_Now(), + stapledOCSPResponse, + infoObject); + if (rv != SECSuccess) { + // Due to buggy servers that will staple expired OCSP responses + // (see for example http://trac.nginx.org/nginx/ticket/425), + // don't terminate the connection if the stapled response is expired. + // We will fall back to fetching revocation information. + PRErrorCode ocspErrorCode = PR_GetError(); + if (ocspErrorCode != SEC_ERROR_OCSP_OLD_RESPONSE) { + // stapled OCSP response present but invalid for some reason + Telemetry::Accumulate(Telemetry::SSL_OCSP_STAPLING, 4); + return rv; + } else { + // stapled OCSP response present but expired + Telemetry::Accumulate(Telemetry::SSL_OCSP_STAPLING, 3); + } } else { - // stapled OCSP response present but expired - Telemetry::Accumulate(Telemetry::SSL_OCSP_STAPLING, 3); + // stapled OCSP response present and good + Telemetry::Accumulate(Telemetry::SSL_OCSP_STAPLING, 1); } } else { - // stapled OCSP response present and good - Telemetry::Accumulate(Telemetry::SSL_OCSP_STAPLING, 1); - } - } else { - // no stapled OCSP response - Telemetry::Accumulate(Telemetry::SSL_OCSP_STAPLING, 2); + // no stapled OCSP response + Telemetry::Accumulate(Telemetry::SSL_OCSP_STAPLING, 2); - uint32_t reasonsForNotFetching = 0; + uint32_t reasonsForNotFetching = 0; - char* ocspURI = CERT_GetOCSPAuthorityInfoAccessLocation(cert); - if (!ocspURI) { - reasonsForNotFetching |= 1; // invalid/missing OCSP URI - } else { - if (std::strncmp(ocspURI, "http://", 7)) { // approximation + char* ocspURI = CERT_GetOCSPAuthorityInfoAccessLocation(cert); + if (!ocspURI) { reasonsForNotFetching |= 1; // invalid/missing OCSP URI + } else { + if (std::strncmp(ocspURI, "http://", 7)) { // approximation + reasonsForNotFetching |= 1; // invalid/missing OCSP URI + } + PORT_Free(ocspURI); } - PORT_Free(ocspURI); - } - if (!certVerifier.mOCSPDownloadEnabled) { - reasonsForNotFetching |= 2; - } + if (!certVerifier.mOCSPDownloadEnabled) { + reasonsForNotFetching |= 2; + } - Telemetry::Accumulate(Telemetry::SSL_OCSP_MAY_FETCH, - reasonsForNotFetching); + Telemetry::Accumulate(Telemetry::SSL_OCSP_MAY_FETCH, + reasonsForNotFetching); + } } // We want to avoid storing any intermediate cert information when browsing diff --git a/security/manager/ssl/src/nsNSSCertificateDB.cpp b/security/manager/ssl/src/nsNSSCertificateDB.cpp index 28694a641962..a4b412b7cf42 100644 --- a/security/manager/ssl/src/nsNSSCertificateDB.cpp +++ b/security/manager/ssl/src/nsNSSCertificateDB.cpp @@ -1800,3 +1800,25 @@ nsNSSCertificateDB::VerifyCertNow(nsIX509Cert* aCert, return NS_OK; } + +NS_IMETHODIMP +nsNSSCertificateDB::ClearOCSPCache() +{ + nsNSSShutDownPreventionLock locker; + if (isAlreadyShutDown()) { + return NS_ERROR_NOT_AVAILABLE; + } + + RefPtr certVerifier(GetDefaultCertVerifier()); + NS_ENSURE_TRUE(certVerifier, NS_ERROR_FAILURE); + if (certVerifier->mImplementation == CertVerifier::insanity) { + certVerifier->ClearOCSPCache(); + } else { + SECStatus srv = CERT_ClearOCSPCache(); + if (srv != SECSuccess) { + return MapSECStatus(srv); + } + } + + return NS_OK; +} diff --git a/security/manager/ssl/src/nsNSSComponent.cpp b/security/manager/ssl/src/nsNSSComponent.cpp index 2421956b4f9a..7db1ac1414ae 100644 --- a/security/manager/ssl/src/nsNSSComponent.cpp +++ b/security/manager/ssl/src/nsNSSComponent.cpp @@ -997,6 +997,18 @@ void nsNSSComponent::setValidationOptions(bool isInitialSetting, #endif odc, osc, ogc); + // insanity::pkix has its own OCSP cache, so disable the NSS cache + // if appropriate. + if (certVerifierImplementation == CertVerifier::insanity) { + // Using -1 disables the cache. The other arguments are the default + // values and aren't exposed by the API. + CERT_OCSPCacheSettings(-1, 1*60*60L, 24*60*60L); + } else { + // Using 1000 enables the cache with the default size of 1000. Again, + // these values are not exposed by the API. + CERT_OCSPCacheSettings(1000, 1*60*60L, 24*60*60L); + } + CERT_ClearOCSPCache(); } diff --git a/security/manager/ssl/tests/gtest/OCSPCacheTest.cpp b/security/manager/ssl/tests/gtest/OCSPCacheTest.cpp new file mode 100644 index 000000000000..f21282d418a5 --- /dev/null +++ b/security/manager/ssl/tests/gtest/OCSPCacheTest.cpp @@ -0,0 +1,250 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "CertVerifier.h" +#include "OCSPCache.h" +#include "nss.h" +#include "prprf.h" +#include "secerr.h" + +#include "gtest/gtest.h" + +const int MaxCacheEntries = 1024; + +class OCSPCacheTest : public ::testing::Test +{ + protected: + static void SetUpTestCase() + { + NSS_NoDB_Init(nullptr); + mozilla::psm::InitCertVerifierLog(); + } + + mozilla::psm::OCSPCache cache; +}; + +// Makes a fake certificate with just the fields we need for testing here. +// (And those values are almost entirely bogus.) +// stackCert should be stack-allocated memory. +static void +MakeFakeCert(CERTCertificate* stackCert, const char* subjectValue, + const char* issuerValue, const char* serialNumberValue, + const char* publicKeyValue) +{ + stackCert->derSubject.data = (unsigned char*)subjectValue; + stackCert->derSubject.len = strlen(subjectValue); + stackCert->derIssuer.data = (unsigned char*)issuerValue; + stackCert->derIssuer.len = strlen(issuerValue); + stackCert->serialNumber.data = (unsigned char*)serialNumberValue; + stackCert->serialNumber.len = strlen(serialNumberValue); + stackCert->derPublicKey.data = (unsigned char*)publicKeyValue; + stackCert->derPublicKey.len = strlen(publicKeyValue); + CERTName *subject = CERT_AsciiToName(subjectValue); // TODO: this will leak... + ASSERT_TRUE(subject); + stackCert->subject.arena = subject->arena; + stackCert->subject.rdns = subject->rdns; +} + +static void +PutAndGet(mozilla::psm::OCSPCache& cache, CERTCertificate* subject, + CERTCertificate* issuer, + PRErrorCode error, PRTime time) +{ + // The first time is thisUpdate. The second is validUntil. + // The caller is expecting the validUntil returned with Get + // to be equal to the passed-in time. Since these values will + // be different in practice, make thisUpdate less than validUntil. + ASSERT_TRUE(time >= 10); + SECStatus rv = cache.Put(subject, issuer, error, time - 10, time); + ASSERT_TRUE(rv == SECSuccess); + PRErrorCode errorOut; + PRTime timeOut; + ASSERT_TRUE(cache.Get(subject, issuer, errorOut, timeOut)); + ASSERT_TRUE(error == errorOut && time == timeOut); +} + +TEST_F(OCSPCacheTest, TestPutAndGet) +{ + SCOPED_TRACE(""); + CERTCertificate subject; + MakeFakeCert(&subject, "CN=subject1", "CN=issuer1", "001", "key001"); + CERTCertificate issuer; + MakeFakeCert(&issuer, "CN=issuer1", "CN=issuer1", "000", "key000"); + PutAndGet(cache, &subject, &issuer, 0, PR_Now()); + PRErrorCode errorOut; + PRTime timeOut; + ASSERT_FALSE(cache.Get(&issuer, &issuer, errorOut, timeOut)); +} + +TEST_F(OCSPCacheTest, TestVariousGets) +{ + SCOPED_TRACE(""); + CERTCertificate issuer; + MakeFakeCert(&issuer, "CN=issuer1", "CN=issuer1", "000", "key000"); + PRTime timeIn = PR_Now(); + for (int i = 0; i < MaxCacheEntries; i++) { + CERTCertificate subject; + char subjectBuf[64]; + PR_snprintf(subjectBuf, sizeof(subjectBuf), "CN=subject%04d", i); + char serialBuf[8]; + PR_snprintf(serialBuf, sizeof(serialBuf), "%04d", i); + MakeFakeCert(&subject, subjectBuf, "CN=issuer1", serialBuf, "key000"); + PutAndGet(cache, &subject, &issuer, 0, timeIn + i); + } + CERTCertificate subject; + // This will be at the end of the list in the cache + PRErrorCode errorOut; + PRTime timeOut; + MakeFakeCert(&subject, "CN=subject0000", "CN=issuer1", "0000", "key000"); + ASSERT_TRUE(cache.Get(&subject, &issuer, errorOut, timeOut)); + ASSERT_TRUE(errorOut == 0 && timeOut == timeIn); + // Once we access it, it goes to the front + ASSERT_TRUE(cache.Get(&subject, &issuer, errorOut, timeOut)); + ASSERT_TRUE(errorOut == 0 && timeOut == timeIn); + MakeFakeCert(&subject, "CN=subject0512", "CN=issuer1", "0512", "key000"); + // This will be in the middle + ASSERT_TRUE(cache.Get(&subject, &issuer, errorOut, timeOut)); + ASSERT_TRUE(errorOut == 0 && timeOut == timeIn + 512); + ASSERT_TRUE(cache.Get(&subject, &issuer, errorOut, timeOut)); + ASSERT_TRUE(errorOut == 0 && timeOut == timeIn + 512); + // We've never seen this certificate + MakeFakeCert(&subject, "CN=subject1111", "CN=issuer1", "1111", "key000"); + ASSERT_FALSE(cache.Get(&subject, &issuer, errorOut, timeOut)); +} + +TEST_F(OCSPCacheTest, TestEviction) +{ + SCOPED_TRACE(""); + CERTCertificate issuer; + PRTime timeIn = PR_Now(); + MakeFakeCert(&issuer, "CN=issuer1", "CN=issuer1", "000", "key000"); + // By putting more distinct entries in the cache than it can hold, + // we cause the least recently used entry to be evicted. + for (int i = 0; i < MaxCacheEntries + 1; i++) { + CERTCertificate subject; + char subjectBuf[64]; + PR_snprintf(subjectBuf, sizeof(subjectBuf), "CN=subject%04d", i); + char serialBuf[8]; + PR_snprintf(serialBuf, sizeof(serialBuf), "%04d", i); + MakeFakeCert(&subject, subjectBuf, "CN=issuer1", serialBuf, "key000"); + PutAndGet(cache, &subject, &issuer, 0, timeIn + i); + } + CERTCertificate evictedSubject; + MakeFakeCert(&evictedSubject, "CN=subject0000", "CN=issuer1", "0000", "key000"); + PRErrorCode errorOut; + PRTime timeOut; + ASSERT_FALSE(cache.Get(&evictedSubject, &issuer, errorOut, timeOut)); +} + +TEST_F(OCSPCacheTest, TestNoEvictionForRevokedResponses) +{ + SCOPED_TRACE(""); + CERTCertificate issuer; + MakeFakeCert(&issuer, "CN=issuer1", "CN=issuer1", "000", "key000"); + CERTCertificate notEvictedSubject; + MakeFakeCert(¬EvictedSubject, "CN=subject0000", "CN=issuer1", "0000", "key000"); + PRTime timeIn = PR_Now(); + PutAndGet(cache, ¬EvictedSubject, &issuer, SEC_ERROR_REVOKED_CERTIFICATE, timeIn); + // By putting more distinct entries in the cache than it can hold, + // we cause the least recently used entry that isn't revoked to be evicted. + for (int i = 1; i < MaxCacheEntries + 1; i++) { + CERTCertificate subject; + char subjectBuf[64]; + PR_snprintf(subjectBuf, sizeof(subjectBuf), "CN=subject%04d", i); + char serialBuf[8]; + PR_snprintf(serialBuf, sizeof(serialBuf), "%04d", i); + MakeFakeCert(&subject, subjectBuf, "CN=issuer1", serialBuf, "key000"); + PutAndGet(cache, &subject, &issuer, 0, timeIn + i); + } + PRErrorCode errorOut; + PRTime timeOut; + ASSERT_TRUE(cache.Get(¬EvictedSubject, &issuer, errorOut, timeOut)); + ASSERT_TRUE(errorOut == SEC_ERROR_REVOKED_CERTIFICATE && timeOut == timeIn); + CERTCertificate evictedSubject; + MakeFakeCert(&evictedSubject, "CN=subject0001", "CN=issuer1", "0001", "key000"); + ASSERT_FALSE(cache.Get(&evictedSubject, &issuer, errorOut, timeOut)); +} + +TEST_F(OCSPCacheTest, TestEverythingIsRevoked) +{ + SCOPED_TRACE(""); + CERTCertificate issuer; + MakeFakeCert(&issuer, "CN=issuer1", "CN=issuer1", "000", "key000"); + PRTime timeIn = PR_Now(); + // Fill up the cache with revoked responses. + for (int i = 0; i < MaxCacheEntries; i++) { + CERTCertificate subject; + char subjectBuf[64]; + PR_snprintf(subjectBuf, sizeof(subjectBuf), "CN=subject%04d", i); + char serialBuf[8]; + PR_snprintf(serialBuf, sizeof(serialBuf), "%04d", i); + MakeFakeCert(&subject, subjectBuf, "CN=issuer1", serialBuf, "key000"); + PutAndGet(cache, &subject, &issuer, SEC_ERROR_REVOKED_CERTIFICATE, timeIn + i); + } + CERTCertificate goodSubject; + MakeFakeCert(&goodSubject, "CN=subject1025", "CN=issuer1", "1025", "key000"); + // This will "succeed", allowing verification to continue. However, + // nothing was actually put in the cache. + SECStatus result = cache.Put(&goodSubject, &issuer, 0, timeIn + 1025 - 50, + timeIn + 1025); + ASSERT_TRUE(result == SECSuccess); + PRErrorCode errorOut; + PRTime timeOut; + ASSERT_FALSE(cache.Get(&goodSubject, &issuer, errorOut, timeOut)); + + CERTCertificate revokedSubject; + MakeFakeCert(&revokedSubject, "CN=subject1026", "CN=issuer1", "1026", "key000"); + // This will fail, causing verification to fail. + result = cache.Put(&revokedSubject, &issuer, SEC_ERROR_REVOKED_CERTIFICATE, + timeIn + 1026 - 50, timeIn + 1026); + PRErrorCode error = PR_GetError(); + ASSERT_TRUE(result == SECFailure); + ASSERT_TRUE(error == SEC_ERROR_REVOKED_CERTIFICATE); +} + +TEST_F(OCSPCacheTest, VariousIssuers) +{ + SCOPED_TRACE(""); + CERTCertificate issuer1; + MakeFakeCert(&issuer1, "CN=issuer1", "CN=issuer1", "000", "key000"); + CERTCertificate issuer2; + MakeFakeCert(&issuer2, "CN=issuer2", "CN=issuer2", "000", "key001"); + CERTCertificate issuer3; + // Note: same CN as issuer1 + MakeFakeCert(&issuer3, "CN=issuer1", "CN=issuer3", "000", "key003"); + CERTCertificate subject; + MakeFakeCert(&subject, "CN=subject", "CN=issuer1", "001", "key002"); + PRTime timeIn = PR_Now(); + PutAndGet(cache, &subject, &issuer1, 0, timeIn); + PRErrorCode errorOut; + PRTime timeOut; + ASSERT_TRUE(cache.Get(&subject, &issuer1, errorOut, timeOut)); + ASSERT_TRUE(errorOut == 0 && timeOut == timeIn); + ASSERT_FALSE(cache.Get(&subject, &issuer2, errorOut, timeOut)); + ASSERT_FALSE(cache.Get(&subject, &issuer3, errorOut, timeOut)); +} + +TEST_F(OCSPCacheTest, Times) +{ + SCOPED_TRACE(""); + CERTCertificate subject; + MakeFakeCert(&subject, "CN=subject1", "CN=issuer1", "001", "key001"); + CERTCertificate issuer; + MakeFakeCert(&issuer, "CN=issuer1", "CN=issuer1", "000", "key000"); + PutAndGet(cache, &subject, &issuer, SEC_ERROR_OCSP_UNKNOWN_CERT, 100); + PutAndGet(cache, &subject, &issuer, 0, 200); + // This should not override the more recent entry. + SECStatus rv = cache.Put(&subject, &issuer, SEC_ERROR_OCSP_UNKNOWN_CERT, 100, 100); + ASSERT_TRUE(rv == SECSuccess); + PRErrorCode errorOut; + PRTime timeOut; + ASSERT_TRUE(cache.Get(&subject, &issuer, errorOut, timeOut)); + // Here we see the more recent time. + ASSERT_TRUE(errorOut == 0 && timeOut == 200); + + // SEC_ERROR_REVOKED_CERTIFICATE overrides everything + PutAndGet(cache, &subject, &issuer, SEC_ERROR_REVOKED_CERTIFICATE, 50); +} diff --git a/security/manager/ssl/tests/gtest/moz.build b/security/manager/ssl/tests/gtest/moz.build index 38ee0b2449d2..1e77b651be96 100644 --- a/security/manager/ssl/tests/gtest/moz.build +++ b/security/manager/ssl/tests/gtest/moz.build @@ -9,16 +9,14 @@ LIBRARY_NAME = 'ssltest' LIBXUL_LIBRARY = True SOURCES += [ - 'TLSIntoleranceTest.cpp', + 'OCSPCacheTest.cpp', + 'TLSIntoleranceTest.cpp', ] LOCAL_INCLUDES += [ + '../../../../certverifier', '../../../../insanity/include', -] - -include('/ipc/chromium/chromium-config.mozbuild') - -LOCAL_INCLUDES += [ '/security/manager/ssl/src', ] +include('/ipc/chromium/chromium-config.mozbuild') diff --git a/security/manager/ssl/tests/unit/head_psm.js b/security/manager/ssl/tests/unit/head_psm.js index a8374985cbcb..ea8433b96bd9 100644 --- a/security/manager/ssl/tests/unit/head_psm.js +++ b/security/manager/ssl/tests/unit/head_psm.js @@ -22,6 +22,7 @@ const SEC_ERROR_BASE = Ci.nsINSSErrorsService.NSS_SEC_ERROR_BASE; const SSL_ERROR_BASE = Ci.nsINSSErrorsService.NSS_SSL_ERROR_BASE; // Sort in numerical order +const SEC_ERROR_BAD_DER = SEC_ERROR_BASE + 9; const SEC_ERROR_EXPIRED_CERTIFICATE = SEC_ERROR_BASE + 11; const SEC_ERROR_REVOKED_CERTIFICATE = SEC_ERROR_BASE + 12; // -8180 const SEC_ERROR_UNKNOWN_ISSUER = SEC_ERROR_BASE + 13; @@ -118,11 +119,9 @@ function _getLibraryFunctionWithNoArguments(functionName, libraryName) { } function clearOCSPCache() { - let CERT_ClearOCSPCache = - _getLibraryFunctionWithNoArguments("CERT_ClearOCSPCache", "nss3"); - if (CERT_ClearOCSPCache() != 0) { - throw "Failed to clear OCSP cache"; - } + let certdb = Cc["@mozilla.org/security/x509certdb;1"] + .getService(Ci.nsIX509CertDB); + certdb.clearOCSPCache(); } function clearSessionCache() { diff --git a/security/manager/ssl/tests/unit/test_ev_certs.js b/security/manager/ssl/tests/unit/test_ev_certs.js index 9b9b386de943..ee440f4b3dbc 100644 --- a/security/manager/ssl/tests/unit/test_ev_certs.js +++ b/security/manager/ssl/tests/unit/test_ev_certs.js @@ -223,12 +223,10 @@ function add_tests_in_mode(useInsanity) let error = certdb.verifyCertNow(cert, certificateUsageSSLServer, flags, verifiedChain, hasEVPolicy); - // XXX(bug 915932): Without an OCSP cache, local-only validation of EV - // certs will always fail due to lack of an OCSP. - do_check_eq(hasEVPolicy.value, isDebugBuild && !useInsanity); + do_check_eq(hasEVPolicy.value, isDebugBuild); do_check_eq(error, - useInsanity ? SEC_ERROR_POLICY_VALIDATION_FAILED - : (isDebugBuild ? 0 + isDebugBuild ? 0 + : (useInsanity ? SEC_ERROR_POLICY_VALIDATION_FAILED : SEC_ERROR_EXTENSION_NOT_FOUND)); failingOcspResponder.stop(run_next_test); }); diff --git a/security/manager/ssl/tests/unit/test_ocsp_caching.js b/security/manager/ssl/tests/unit/test_ocsp_caching.js index c8ee2b6c78a8..bfd27c1b4e8a 100644 --- a/security/manager/ssl/tests/unit/test_ocsp_caching.js +++ b/security/manager/ssl/tests/unit/test_ocsp_caching.js @@ -40,6 +40,20 @@ function run_test() { }); ocspResponder.start(8080); + add_tests_in_mode(true); + add_tests_in_mode(false); + + add_test(function() { ocspResponder.stop(run_next_test); }); + run_next_test(); +} + +function add_tests_in_mode(useInsanity) { + add_test(function () { + Services.prefs.setBoolPref("security.use_insanity_verification", + useInsanity); + run_next_test(); + }); + // This test assumes that OCSPStaplingServer uses the same cert for // ocsp-stapling-unknown.example.com and ocsp-stapling-none.example.com. @@ -93,10 +107,13 @@ function run_test() { clearSessionCache); add_test(function() { do_check_eq(gFetchCount, 1); run_next_test(); }); - // The error entry will prevent a fetch from happening for a while. - add_connection_test("ocsp-stapling-none.example.com", Cr.NS_OK, - clearSessionCache); - add_test(function() { do_check_eq(gFetchCount, 1); run_next_test(); }); + // TODO(bug 977865): implement this for insanity + if (!useInsanity) { + // The error entry will prevent a fetch from happening for a while. + add_connection_test("ocsp-stapling-none.example.com", Cr.NS_OK, + clearSessionCache); + add_test(function() { do_check_eq(gFetchCount, 1); run_next_test(); }); + } // The error entry must not prevent a stapled OCSP response from being // honored. @@ -107,7 +124,6 @@ function run_test() { //--------------------------------------------------------------------------- - add_test(function() { ocspResponder.stop(run_next_test); }); - - run_next_test(); + // Reset state + add_test(function() { clearOCSPCache(); gFetchCount = 0; run_next_test(); }); } diff --git a/security/manager/ssl/tests/unit/test_ocsp_required.js b/security/manager/ssl/tests/unit/test_ocsp_required.js index 31cee9573969..4f2c74e1007d 100644 --- a/security/manager/ssl/tests/unit/test_ocsp_required.js +++ b/security/manager/ssl/tests/unit/test_ocsp_required.js @@ -53,8 +53,8 @@ function add_tests_in_mode(useInsanity) add_connection_test("ocsp-stapling-none.example.com", getXPCOMStatusFromNSS(SEC_ERROR_OCSP_BAD_SIGNATURE)); add_test(function () { - // XXX(bug 915932): special case for insanity::pkix due to the temporary - // lack of an OCSP cache. + // TODO(bug 977865): insanity::pkix keeps requesting responses from + // failing responders do_check_eq(gOCSPRequestCount, useInsanity ? 2 : 1); gOCSPRequestCount = 0; run_next_test(); diff --git a/security/manager/ssl/tests/unit/test_ocsp_stapling.js b/security/manager/ssl/tests/unit/test_ocsp_stapling.js index 323a9052affe..8267116dffae 100644 --- a/security/manager/ssl/tests/unit/test_ocsp_stapling.js +++ b/security/manager/ssl/tests/unit/test_ocsp_stapling.js @@ -52,7 +52,9 @@ function add_tests_in_mode(useInsanity, certDB, otherTestCA) { getXPCOMStatusFromNSS(SEC_ERROR_REVOKED_CERTIFICATE), true); // SEC_ERROR_OCSP_INVALID_SIGNING_CERT vs SEC_ERROR_OCSP_UNAUTHORIZED_RESPONSE - // depends on whether the CA that signed the response is a trusted CA. + // depends on whether the CA that signed the response is a trusted CA + // (but only with the classic implementation - insanity::pkix always + // results in the error SEC_ERROR_OCSP_INVALID_SIGNING_CERT). // This stapled response is from a CA that is untrusted and did not issue // the server's certificate. @@ -93,8 +95,11 @@ function add_tests_in_mode(useInsanity, certDB, otherTestCA) { true); add_ocsp_test("ocsp-stapling-unknown.example.com", getXPCOMStatusFromNSS(SEC_ERROR_OCSP_UNKNOWN_CERT), true); + // TODO(bug 977870): this should not result in SEC_ERROR_BAD_DER add_ocsp_test("ocsp-stapling-good-other.example.com", - getXPCOMStatusFromNSS(SEC_ERROR_OCSP_UNKNOWN_CERT), true); + getXPCOMStatusFromNSS( + useInsanity ? SEC_ERROR_BAD_DER + : SEC_ERROR_OCSP_UNKNOWN_CERT), true); // If the server doesn't staple an OCSP response, we continue as normal // (this means that even though stapling is enabled, we expect an OCSP // request). @@ -106,8 +111,11 @@ function add_tests_in_mode(useInsanity, certDB, otherTestCA) { Services.prefs.setBoolPref("security.ssl.enable_ocsp_stapling", true); } ); + // TODO(bug 977870): this should not result in SEC_ERROR_BAD_DER add_ocsp_test("ocsp-stapling-empty.example.com", - getXPCOMStatusFromNSS(SEC_ERROR_OCSP_MALFORMED_RESPONSE), true); + getXPCOMStatusFromNSS( + useInsanity ? SEC_ERROR_BAD_DER + : SEC_ERROR_OCSP_MALFORMED_RESPONSE), true); // ocsp-stapling-expired.example.com and // ocsp-stapling-expired-fresh-ca.example.com are handled in // test_ocsp_stapling_expired.js @@ -118,11 +126,11 @@ function check_ocsp_stapling_telemetry() { .getService(Ci.nsITelemetry) .getHistogramById("SSL_OCSP_STAPLING") .snapshot(); - do_check_eq(histogram.counts[0], 2 * 0); // histogram bucket 0 is unused - do_check_eq(histogram.counts[1], 2 * 1); // 1 connection with a good response - do_check_eq(histogram.counts[2], 2 * 14); // 14 connections with no stapled resp. - do_check_eq(histogram.counts[3], 2 * 0); // 0 connections with an expired response - do_check_eq(histogram.counts[4], 2 * 11); // 11 connections with bad responses + do_check_eq(histogram.counts[0], 0); // histogram bucket 0 is unused + do_check_eq(histogram.counts[1], 1); // 1 connection with a good response + do_check_eq(histogram.counts[2], 14); // 14 connections with no stapled resp. + do_check_eq(histogram.counts[3], 0); // 0 connections with an expired response + do_check_eq(histogram.counts[4], 11); // 11 connections with bad responses run_next_test(); } diff --git a/security/manager/ssl/tests/unit/test_ocsp_stapling_expired.js b/security/manager/ssl/tests/unit/test_ocsp_stapling_expired.js index f2a1a934af29..dbed60d66939 100644 --- a/security/manager/ssl/tests/unit/test_ocsp_stapling_expired.js +++ b/security/manager/ssl/tests/unit/test_ocsp_stapling_expired.js @@ -107,10 +107,10 @@ function check_ocsp_stapling_telemetry() { .getService(Ci.nsITelemetry) .getHistogramById("SSL_OCSP_STAPLING") .snapshot(); - do_check_eq(histogram.counts[0], 2 * 0); // histogram bucket 0 is unused - do_check_eq(histogram.counts[1], 2 * 0); // 0 connections with a good response - do_check_eq(histogram.counts[2], 2 * 0); // 0 connections with no stapled resp. - do_check_eq(histogram.counts[3], 2 * 9); // 8 connections with an expired response - do_check_eq(histogram.counts[4], 2 * 0); // 0 connections with bad responses + do_check_eq(histogram.counts[0], 0); // histogram bucket 0 is unused + do_check_eq(histogram.counts[1], 0); // 0 connections with a good response + do_check_eq(histogram.counts[2], 0); // 0 connections with no stapled resp. + do_check_eq(histogram.counts[3], 9); // 9 connections with an expired response + do_check_eq(histogram.counts[4], 0); // 0 connections with bad responses run_next_test(); }