From 9d23ee7fc75030f75b971ab2c768992c4b6d904e Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 27 Sep 2013 19:53:36 -0700 Subject: [PATCH] Bug 891066, Part 8: Add stapled OCSP response to CertVerifier, r=cviecco --HG-- extra : rebase_source : ffe0762228d1217cb51e2f8fad2e0605d7d61344 extra : source : f721d60b6bf74467381590457ce3542f83a2f43a --- security/certverifier/CertVerifier.cpp | 6 ++- security/certverifier/CertVerifier.h | 2 + security/certverifier/NSSCertDBTrustDomain.h | 1 + .../ssl/src/SSLServerCertVerification.cpp | 52 ++++++++++++------- security/manager/ssl/src/nsCMS.cpp | 2 +- security/manager/ssl/src/nsNSSCertificate.cpp | 6 +-- .../manager/ssl/src/nsNSSCertificateDB.cpp | 9 ++-- .../manager/ssl/src/nsUsageArrayHelper.cpp | 2 +- 8 files changed, 51 insertions(+), 29 deletions(-) diff --git a/security/certverifier/CertVerifier.cpp b/security/certverifier/CertVerifier.cpp index 7aafe74983c9..d63440b4720b 100644 --- a/security/certverifier/CertVerifier.cpp +++ b/security/certverifier/CertVerifier.cpp @@ -12,6 +12,7 @@ #include "ExtendedValidation.h" #include "NSSCertDBTrustDomain.h" #include "cert.h" +#include "ocsp.h" #include "secerr.h" #include "prerror.h" #include "sslerr.h" @@ -149,6 +150,7 @@ destroyCertListThatShouldNotExist(CERTCertList** certChain) SECStatus CertVerifier::VerifyCert(CERTCertificate* cert, + /*optional*/ const SECItem* stapledOCSPResponse, const SECCertificateUsage usage, const PRTime time, void* pinArg, @@ -467,6 +469,7 @@ pkix_done: SECStatus CertVerifier::VerifySSLServerCert(CERTCertificate* peerCert, + /*optional*/ const SECItem* stapledOCSPResponse, PRTime time, /*optional*/ void* pinarg, const char* hostname, @@ -492,7 +495,8 @@ CertVerifier::VerifySSLServerCert(CERTCertificate* peerCert, } ScopedCERTCertList validationChain; - SECStatus rv = VerifyCert(peerCert, certificateUsageSSLServer, time, + SECStatus rv = VerifyCert(peerCert, stapledOCSPResponse, + certificateUsageSSLServer, time, pinarg, 0, &validationChain, evOidPolicy); if (rv != SECSuccess) { return rv; diff --git a/security/certverifier/CertVerifier.h b/security/certverifier/CertVerifier.h index 092e6871d5a0..5eec6ed37e6e 100644 --- a/security/certverifier/CertVerifier.h +++ b/security/certverifier/CertVerifier.h @@ -24,6 +24,7 @@ public: // *evOidPolicy == SEC_OID_UNKNOWN means the cert is NOT EV // Only one usage per verification is supported. SECStatus VerifyCert(CERTCertificate* cert, + /*optional*/ const SECItem* stapledOCSPResponse, const SECCertificateUsage usage, const PRTime time, void* pinArg, @@ -34,6 +35,7 @@ public: SECStatus VerifySSLServerCert( CERTCertificate* peerCert, + /*optional*/ const SECItem* stapledOCSPResponse, PRTime time, /*optional*/ void* pinarg, const char* hostname, diff --git a/security/certverifier/NSSCertDBTrustDomain.h b/security/certverifier/NSSCertDBTrustDomain.h index 2af51a8e9c75..7d8ee6196148 100644 --- a/security/certverifier/NSSCertDBTrustDomain.h +++ b/security/certverifier/NSSCertDBTrustDomain.h @@ -7,6 +7,7 @@ #ifndef mozilla_psm__NSSCertDBTrustDomain_h #define mozilla_psm__NSSCertDBTrustDomain_h +#include "insanity/pkixtypes.h" #include "secmodt.h" #include "CertVerifier.h" diff --git a/security/manager/ssl/src/SSLServerCertVerification.cpp b/security/manager/ssl/src/SSLServerCertVerification.cpp index 9146660b1901..b6a21fcc40a7 100644 --- a/security/manager/ssl/src/SSLServerCertVerification.cpp +++ b/security/manager/ssl/src/SSLServerCertVerification.cpp @@ -473,6 +473,7 @@ CreateCertErrorRunnable(CertVerifier& certVerifier, PRErrorCode defaultErrorCodeToReport, TransportSecurityInfo* infoObject, CERTCertificate* cert, + const SECItem* stapledOCSPResponse, const void* fdForLogging, uint32_t providerFlags, PRTime now) @@ -518,7 +519,10 @@ CreateCertErrorRunnable(CertVerifier& certVerifier, CERTVerifyLogContentsCleaner verify_log_cleaner(verify_log); verify_log->arena = log_arena; - srv = certVerifier.VerifyCert(cert, certificateUsageSSLServer, now, + // XXX TODO: convert to VerifySSLServerCert + // XXX TODO: get rid of error log + srv = certVerifier.VerifyCert(cert, stapledOCSPResponse, + certificateUsageSSLServer, now, infoObject, 0, nullptr, nullptr, verify_log); // We ignore the result code of the cert verification. @@ -622,7 +626,8 @@ public: TransportSecurityInfo* infoObject, CERTCertificate* serverCert, SECItem* stapledOCSPResponse, - uint32_t providerFlags); + uint32_t providerFlags, + PRTime time); private: NS_DECL_NSIRUNNABLE @@ -632,12 +637,14 @@ private: TransportSecurityInfo* infoObject, CERTCertificate* cert, SECItem* stapledOCSPResponse, - uint32_t providerFlags); + uint32_t providerFlags, + PRTime time); const RefPtr mCertVerifier; const void* const mFdForLogging; const RefPtr mInfoObject; const insanity::pkix::ScopedCERTCertificate mCert; const uint32_t mProviderFlags; + const PRTime mTime; const TimeStamp mJobStartTime; const ScopedSECItem mStapledOCSPResponse; }; @@ -645,12 +652,13 @@ private: SSLServerCertVerificationJob::SSLServerCertVerificationJob( const RefPtr& certVerifier, const void* fdForLogging, TransportSecurityInfo* infoObject, CERTCertificate* cert, - SECItem* stapledOCSPResponse, uint32_t providerFlags) + SECItem* stapledOCSPResponse, uint32_t providerFlags, PRTime time) : mCertVerifier(certVerifier) , mFdForLogging(fdForLogging) , mInfoObject(infoObject) , mCert(CERT_DupCertificate(cert)) , mProviderFlags(providerFlags) + , mTime(time) , mJobStartTime(TimeStamp::Now()) , mStapledOCSPResponse(SECITEM_DupItem(stapledOCSPResponse)) { @@ -727,12 +735,13 @@ BlockServerCertChangeForSpdy(nsNSSSocketInfo* infoObject, SECStatus AuthCertificate(CertVerifier& certVerifier, TransportSecurityInfo* infoObject, CERTCertificate* cert, SECItem* stapledOCSPResponse, - uint32_t providerFlags) + uint32_t providerFlags, PRTime time) { - MOZ_ASSERT(infoObject); - MOZ_ASSERT(cert); - + MOZ_ASSERT(infoObject); + MOZ_ASSERT(cert); + SECStatus rv; + if (stapledOCSPResponse) { CERTCertDBHandle* handle = CERT_GetDefaultCertDB(); rv = CERT_CacheOCSPResponseFromSideChannel(handle, cert, PR_Now(), @@ -780,14 +789,15 @@ AuthCertificate(CertVerifier& certVerifier, TransportSecurityInfo* infoObject, reasonsForNotFetching); } - // We want to avoid storing any intermediate cert information when browsing - // in private, transient contexts. - bool saveIntermediates = + // We want to avoid storing any intermediate cert information when browsing + // in private, transient contexts. + bool saveIntermediates = !(providerFlags & nsISocketProvider::NO_PERMANENT_STORAGE); insanity::pkix::ScopedCERTCertList certList; SECOidTag evOidPolicy; - rv = certVerifier.VerifySSLServerCert(cert, PR_Now(), infoObject, + rv = certVerifier.VerifySSLServerCert(cert, stapledOCSPResponse, + time, infoObject, infoObject->GetHostNameRaw(), saveIntermediates, nullptr, &evOidPolicy); @@ -846,7 +856,8 @@ SSLServerCertVerificationJob::Dispatch( TransportSecurityInfo* infoObject, CERTCertificate* serverCert, SECItem* stapledOCSPResponse, - uint32_t providerFlags) + uint32_t providerFlags, + PRTime time) { // Runs on the socket transport thread if (!certVerifier || !infoObject || !serverCert) { @@ -858,7 +869,7 @@ SSLServerCertVerificationJob::Dispatch( RefPtr job( new SSLServerCertVerificationJob(certVerifier, fdForLogging, infoObject, serverCert, stapledOCSPResponse, - providerFlags)); + providerFlags, time)); nsresult nrv; if (!gCertVerificationThreadPool) { @@ -920,11 +931,13 @@ SSLServerCertVerificationJob::Run() MOZ_CRASH("Unknown CertVerifier mode"); } + // XXX // Reset the error code here so we can detect if AuthCertificate fails to // set the error code if/when it fails. PR_SetError(0, 0); SECStatus rv = AuthCertificate(*mCertVerifier, mInfoObject, mCert.get(), - mStapledOCSPResponse, mProviderFlags); + mStapledOCSPResponse, mProviderFlags, + mTime); if (rv == SECSuccess) { uint32_t interval = (uint32_t) ((TimeStamp::Now() - mJobStartTime).ToMilliseconds()); RefPtr restart( @@ -945,8 +958,8 @@ SSLServerCertVerificationJob::Run() if (error != 0) { RefPtr runnable( CreateCertErrorRunnable(*mCertVerifier, error, mInfoObject, - mCert.get(), mFdForLogging, mProviderFlags, - PR_Now())); + mCert.get(), mStapledOCSPResponse, + mFdForLogging, mProviderFlags, mTime)); if (!runnable) { // CreateCertErrorRunnable set a new error code error = PR_GetError(); @@ -1070,7 +1083,7 @@ AuthCertificateHook(void* arg, PRFileDesc* fd, PRBool checkSig, PRBool isServer) socketInfo->SetCertVerificationWaiting(); SECStatus rv = SSLServerCertVerificationJob::Dispatch( certVerifier, static_cast(fd), socketInfo, - serverCert, stapledOCSPResponse, providerFlags); + serverCert, stapledOCSPResponse, providerFlags, now); return rv; } @@ -1080,7 +1093,7 @@ AuthCertificateHook(void* arg, PRFileDesc* fd, PRBool checkSig, PRBool isServer) // a non-blocking socket. SECStatus rv = AuthCertificate(*certVerifier, socketInfo, serverCert, - stapledOCSPResponse, providerFlags); + stapledOCSPResponse, providerFlags, now); if (rv == SECSuccess) { return SECSuccess; } @@ -1089,6 +1102,7 @@ AuthCertificateHook(void* arg, PRFileDesc* fd, PRBool checkSig, PRBool isServer) if (error != 0) { RefPtr runnable( CreateCertErrorRunnable(*certVerifier, error, socketInfo, serverCert, + stapledOCSPResponse, static_cast(fd), providerFlags, now)); if (!runnable) { diff --git a/security/manager/ssl/src/nsCMS.cpp b/security/manager/ssl/src/nsCMS.cpp index d4eff1395206..038b06c003a0 100644 --- a/security/manager/ssl/src/nsCMS.cpp +++ b/security/manager/ssl/src/nsCMS.cpp @@ -264,7 +264,7 @@ nsresult nsCMSMessage::CommonVerifySignature(unsigned char* aDigestData, uint32_ NS_ENSURE_TRUE(certVerifier, NS_ERROR_UNEXPECTED); { - SECStatus srv = certVerifier->VerifyCert(si->cert, + SECStatus srv = certVerifier->VerifyCert(si->cert, nullptr, certificateUsageEmailSigner, PR_Now(), nullptr /*XXX pinarg*/); if (srv != SECSuccess) { diff --git a/security/manager/ssl/src/nsNSSCertificate.cpp b/security/manager/ssl/src/nsNSSCertificate.cpp index 74054a34d387..19805a6ac737 100644 --- a/security/manager/ssl/src/nsNSSCertificate.cpp +++ b/security/manager/ssl/src/nsNSSCertificate.cpp @@ -829,7 +829,7 @@ nsNSSCertificate::GetChain(nsIArray** _rvChain) // We want to test all usages, but we start with server because most of the // time Firefox users care about server certs. - srv = certVerifier->VerifyCert(mCert.get(), + srv = certVerifier->VerifyCert(mCert.get(), nullptr, certificateUsageSSLServer, PR_Now(), nullptr, /*XXX fixme*/ CertVerifier::FLAG_LOCAL_ONLY, @@ -851,7 +851,7 @@ nsNSSCertificate::GetChain(nsIArray** _rvChain) PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, ("pipnss: PKIX attempting chain(%d) for '%s'\n", usage, mCert->nickname)); - srv = certVerifier->VerifyCert(mCert.get(), + srv = certVerifier->VerifyCert(mCert.get(), nullptr, usage, PR_Now(), nullptr, /*XXX fixme*/ CertVerifier::FLAG_LOCAL_ONLY, @@ -1442,7 +1442,7 @@ nsNSSCertificate::hasValidEVOidTag(SECOidTag& resultOidTag, bool& validEV) uint32_t flags = mozilla::psm::CertVerifier::FLAG_LOCAL_ONLY | mozilla::psm::CertVerifier::FLAG_NO_DV_FALLBACK_FOR_EV; - SECStatus rv = certVerifier->VerifyCert(mCert.get(), + SECStatus rv = certVerifier->VerifyCert(mCert.get(), nullptr, certificateUsageSSLServer, PR_Now(), nullptr /* XXX pinarg */, flags, nullptr, &resultOidTag); diff --git a/security/manager/ssl/src/nsNSSCertificateDB.cpp b/security/manager/ssl/src/nsNSSCertificateDB.cpp index 8b904782b9c7..5c42394f81b7 100644 --- a/security/manager/ssl/src/nsNSSCertificateDB.cpp +++ b/security/manager/ssl/src/nsNSSCertificateDB.cpp @@ -626,7 +626,7 @@ nsNSSCertificateDB::ImportEmailCertificate(uint8_t * data, uint32_t length, insanity::pkix::ScopedCERTCertList certChain; - SECStatus rv = certVerifier->VerifyCert(node->cert, + SECStatus rv = certVerifier->VerifyCert(node->cert, nullptr, certificateUsageEmailRecipient, now, ctx, 0, &certChain); @@ -793,7 +793,8 @@ nsNSSCertificateDB::ImportValidCACertsInList(CERTCertList *certList, nsIInterfac !CERT_LIST_END(node,certList); node = CERT_LIST_NEXT(node)) { insanity::pkix::ScopedCERTCertList certChain; - SECStatus rv = certVerifier->VerifyCert(node->cert, certificateUsageVerifyCA, + SECStatus rv = certVerifier->VerifyCert(node->cert, nullptr, + certificateUsageVerifyCA, PR_Now(), ctx, 0, &certChain); if (rv != SECSuccess) { nsCOMPtr certToShow = nsNSSCertificate::Create(node->cert); @@ -1365,7 +1366,7 @@ nsNSSCertificateDB::FindCertByEmailAddress(nsISupports *aToken, const char *aEma !CERT_LIST_END(node, certlist); node = CERT_LIST_NEXT(node)) { - SECStatus srv = certVerifier->VerifyCert(node->cert, + SECStatus srv = certVerifier->VerifyCert(node->cert, nullptr, certificateUsageEmailRecipient, PR_Now(), nullptr /*XXX pinarg*/); if (srv == SECSuccess) { @@ -1714,7 +1715,7 @@ nsNSSCertificateDB::VerifyCertNow(nsIX509Cert* aCert, SECOidTag evOidPolicy; SECStatus srv; - srv = certVerifier->VerifyCert(nssCert, + srv = certVerifier->VerifyCert(nssCert, nullptr, aUsage, PR_Now(), nullptr, // Assume no context aFlags, diff --git a/security/manager/ssl/src/nsUsageArrayHelper.cpp b/security/manager/ssl/src/nsUsageArrayHelper.cpp index 5f45ee49c6fe..b7c66677b01c 100644 --- a/security/manager/ssl/src/nsUsageArrayHelper.cpp +++ b/security/manager/ssl/src/nsUsageArrayHelper.cpp @@ -117,7 +117,7 @@ nsUsageArrayHelper::check(uint32_t previousCheckResult, MOZ_CRASH("unknown cert usage passed to check()"); } - SECStatus rv = certVerifier->VerifyCert(mCert, aCertUsage, + SECStatus rv = certVerifier->VerifyCert(mCert, nullptr, aCertUsage, time, nullptr /*XXX:wincx*/, flags); if (rv == SECSuccess) {