From a34a6509764ae27131aa466ea8f063bf04d8a94d Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Fri, 7 Dec 2012 17:50:43 -0500 Subject: [PATCH] Bug 769288 - Part 5: Close private socket connections when the lsat private browsing instance dies. r=bsmith,mcmanus --- netwerk/base/public/nsASocketHandler.h | 3 + netwerk/base/src/nsSocketTransport2.cpp | 1 + .../base/src/nsSocketTransportService2.cpp | 45 +++++-- netwerk/base/src/nsSocketTransportService2.h | 2 + security/manager/ssl/src/Makefile.in | 7 +- security/manager/ssl/src/PublicSSL.h | 23 ++++ security/manager/ssl/src/SharedSSLState.cpp | 119 ++++++++++++++++++ security/manager/ssl/src/SharedSSLState.h | 16 ++- .../manager/ssl/src/nsCertOverrideService.cpp | 8 +- .../manager/ssl/src/nsNSSCertificateDB.cpp | 7 +- security/manager/ssl/src/nsNSSCertificateDB.h | 2 + security/manager/ssl/src/nsNSSComponent.cpp | 6 - security/manager/ssl/src/nsNSSIOLayer.cpp | 2 + security/manager/ssl/src/nsRecentBadCerts.cpp | 23 +--- security/manager/ssl/src/nsRecentBadCerts.h | 5 - 15 files changed, 218 insertions(+), 51 deletions(-) create mode 100644 security/manager/ssl/src/PublicSSL.h diff --git a/netwerk/base/public/nsASocketHandler.h b/netwerk/base/public/nsASocketHandler.h index ff459bbb2de7..7af6aa20e63a 100644 --- a/netwerk/base/public/nsASocketHandler.h +++ b/netwerk/base/public/nsASocketHandler.h @@ -15,6 +15,7 @@ public: : mCondition(NS_OK) , mPollFlags(0) , mPollTimeout(UINT16_MAX) + , mIsPrivate(false) {} // @@ -42,6 +43,8 @@ public: // uint16_t mPollTimeout; + bool mIsPrivate; + // // called to service a socket // diff --git a/netwerk/base/src/nsSocketTransport2.cpp b/netwerk/base/src/nsSocketTransport2.cpp index a348eb59b117..dd7eafc1f41a 100644 --- a/netwerk/base/src/nsSocketTransport2.cpp +++ b/netwerk/base/src/nsSocketTransport2.cpp @@ -2206,6 +2206,7 @@ NS_IMETHODIMP nsSocketTransport::SetConnectionFlags(uint32_t value) { mConnectionFlags = value; + mIsPrivate = value & nsISocketTransport::NO_PERMANENT_STORAGE; return NS_OK; } diff --git a/netwerk/base/src/nsSocketTransportService2.cpp b/netwerk/base/src/nsSocketTransportService2.cpp index 3bfb5f497c9c..50889d8a1352 100644 --- a/netwerk/base/src/nsSocketTransportService2.cpp +++ b/netwerk/base/src/nsSocketTransportService2.cpp @@ -23,15 +23,7 @@ #include "mozilla/Services.h" #include "mozilla/Preferences.h" #include "mozilla/Likely.h" - - -// XXX: There is no good header file to put these in. :( -namespace mozilla { namespace psm { - -void InitializeSSLServerCertVerificationThreads(); -void StopSSLServerCertVerificationThreads(); - -} } // namespace mozilla::psm +#include "mozilla/PublicSSL.h" using namespace mozilla; using namespace mozilla::net; @@ -470,6 +462,7 @@ nsSocketTransportService::Init() nsCOMPtr obsSvc = services::GetObserverService(); if (obsSvc) { obsSvc->AddObserver(this, "profile-initial-state", false); + obsSvc->AddObserver(this, "last-pb-context-exited", false); } mInitialized = true; @@ -517,6 +510,7 @@ nsSocketTransportService::Shutdown() nsCOMPtr obsSvc = services::GetObserverService(); if (obsSvc) { obsSvc->RemoveObserver(this, "profile-initial-state"); + obsSvc->RemoveObserver(this, "last-pb-context-exited"); } mozilla::net::NetworkActivityMonitor::Shutdown(); @@ -884,9 +878,42 @@ nsSocketTransportService::Observe(nsISupports *subject, return net::NetworkActivityMonitor::Init(blipInterval); } + + if (!strcmp(topic, "last-pb-context-exited")) { + nsCOMPtr ev = + NS_NewRunnableMethod(this, + &nsSocketTransportService::ClosePrivateConnections); + nsresult rv = Dispatch(ev, nsIEventTarget::DISPATCH_NORMAL); + NS_ENSURE_SUCCESS(rv, rv); + } + return NS_OK; } +void +nsSocketTransportService::ClosePrivateConnections() +{ + // Must be called on the socket thread. +#ifdef DEBUG + bool onSTSThread; + IsOnCurrentThread(&onSTSThread); + MOZ_ASSERT(onSTSThread); +#endif + + for (int32_t i = mActiveCount - 1; i >= 0; --i) { + if (mActiveList[i].mHandler->mIsPrivate) { + DetachSocket(mActiveList, &mActiveList[i]); + } + } + for (int32_t i = mIdleCount - 1; i >= 0; --i) { + if (mIdleList[i].mHandler->mIsPrivate) { + DetachSocket(mIdleList, &mIdleList[i]); + } + } + + mozilla::ClearPrivateSSLState(); +} + NS_IMETHODIMP nsSocketTransportService::GetSendBufferSize(int32_t *value) { diff --git a/netwerk/base/src/nsSocketTransportService2.h b/netwerk/base/src/nsSocketTransportService2.h index 7e2045b49470..753b8f7f2ddd 100644 --- a/netwerk/base/src/nsSocketTransportService2.h +++ b/netwerk/base/src/nsSocketTransportService2.h @@ -190,6 +190,8 @@ private: void AnalyzeConnection(nsTArray *data, SocketContext *context, bool aActive); + + void ClosePrivateConnections(); }; extern nsSocketTransportService *gSocketTransportService; diff --git a/security/manager/ssl/src/Makefile.in b/security/manager/ssl/src/Makefile.in index 86028d2e646c..a5ba00289423 100644 --- a/security/manager/ssl/src/Makefile.in +++ b/security/manager/ssl/src/Makefile.in @@ -83,7 +83,6 @@ endif CSRCS += md4.c - EXTRA_DEPS = $(NSS_DEP_LIBS) DEFINES += \ @@ -98,5 +97,11 @@ EXPORTS += \ ScopedNSSTypes.h \ $(NULL) +EXPORTS_NAMESPACES = mozilla + +EXPORTS_mozilla += \ + PublicSSL.h \ + $(NULL) + include $(topsrcdir)/config/rules.mk diff --git a/security/manager/ssl/src/PublicSSL.h b/security/manager/ssl/src/PublicSSL.h new file mode 100644 index 000000000000..738a6e1f12d1 --- /dev/null +++ b/security/manager/ssl/src/PublicSSL.h @@ -0,0 +1,23 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=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/. */ + +#ifndef mozilla_SSL_h +#define mozilla_SSL_h + +namespace mozilla { + +void ClearPrivateSSLState(); + +namespace psm { + +void InitializeSSLServerCertVerificationThreads(); +void StopSSLServerCertVerificationThreads(); + +} //namespace psm +} // namespace mozilla + +#endif + diff --git a/security/manager/ssl/src/SharedSSLState.cpp b/security/manager/ssl/src/SharedSSLState.cpp index 6a4e7b1a493a..3aa3d9b2ae8c 100644 --- a/security/manager/ssl/src/SharedSSLState.cpp +++ b/security/manager/ssl/src/SharedSSLState.cpp @@ -12,8 +12,93 @@ #include "mozilla/Services.h" #include "nsThreadUtils.h" #include "nsCRT.h" +#include "nsServiceManagerUtils.h" +#include "nsRecentBadCerts.h" +#include "PSMRunnable.h" +#include "PublicSSL.h" +#include "ssl.h" +#include "nsNetCID.h" +#include "mozilla/unused.h" + +using mozilla::psm::SyncRunnableBase; +using mozilla::unused; + +namespace { + +static PRInt32 sCertOverrideSvcExists = 0; +static PRInt32 sCertDBExists = 0; + +class MainThreadClearer : public SyncRunnableBase +{ +public: + MainThreadClearer() : mShouldClearSessionCache(false) {} + + void RunOnTargetThread() { + // In some cases it's possible to cause PSM/NSS to initialize while XPCOM shutdown + // is in progress. We want to avoid this, since they do not handle the situation well, + // hence the flags to avoid instantiating the services if they don't already exist. + + bool certOverrideSvcExists = (bool)PR_ATOMIC_SET(&sCertOverrideSvcExists, 0); + if (certOverrideSvcExists) { + unused << PR_ATOMIC_SET(&sCertOverrideSvcExists, 1); + nsCOMPtr icos = do_GetService(NS_CERTOVERRIDE_CONTRACTID); + if (icos) { + icos->ClearValidityOverride( + NS_LITERAL_CSTRING("all:temporary-certificates"), + 0); + } + } + + bool certDBExists = (bool)PR_ATOMIC_SET(&sCertDBExists, 0); + if (certDBExists) { + unused << PR_ATOMIC_SET(&sCertDBExists, 1); + nsCOMPtr certdb = do_GetService(NS_X509CERTDB_CONTRACTID); + if (certdb) { + nsCOMPtr badCerts; + certdb->GetRecentBadCerts(true, getter_AddRefs(badCerts)); + if (badCerts) { + badCerts->ResetStoredCerts(); + } + } + } + + // This needs to be checked on the main thread to avoid racing with NSS + // initialization. + mShouldClearSessionCache = mozilla::psm::PrivateSSLState() && + mozilla::psm::PrivateSSLState()->SocketCreated(); + } + bool mShouldClearSessionCache; +}; + +} // anonymous namespace namespace mozilla { + +void ClearPrivateSSLState() +{ + // This only works if it is called on the socket transport + // service thread immediately after closing all private SSL + // connections. +#ifdef DEBUG + nsresult rv; + nsCOMPtr sts + = do_GetService(NS_SOCKETTRANSPORTSERVICE_CONTRACTID, &rv); + MOZ_ASSERT(NS_SUCCEEDED(rv)); + bool onSTSThread; + rv = sts->IsOnCurrentThread(&onSTSThread); + MOZ_ASSERT(NS_SUCCEEDED(rv) && onSTSThread); +#endif + + RefPtr runnable = new MainThreadClearer; + runnable->DispatchToMainThreadAndWait(); + + // If NSS isn't initialized, this throws an assertion. We guard it by checking if + // the session cache might even have anything worth clearing. + if (runnable->mShouldClearSessionCache) { + SSL_ClearSessionCache(); + } +} + namespace psm { namespace { @@ -46,14 +131,21 @@ PrivateBrowsingObserver::Observe(nsISupports *aSubject, SharedSSLState::SharedSSLState() : mClientAuthRemember(new nsClientAuthRememberService) +, mMutex("SharedSSLState::mMutex") +, mSocketCreated(false) { mIOLayerHelpers.Init(); mClientAuthRemember->Init(); } +SharedSSLState::~SharedSSLState() +{ +} + void SharedSSLState::NotePrivateBrowsingStatus() { + MOZ_ASSERT(NS_IsMainThread(), "Not on main thread"); mObserver = new PrivateBrowsingObserver(this); nsCOMPtr obsSvc = mozilla::services::GetObserverService(); obsSvc->AddObserver(mObserver, "last-pb-context-exited", false); @@ -62,10 +154,25 @@ SharedSSLState::NotePrivateBrowsingStatus() void SharedSSLState::ResetStoredData() { + MOZ_ASSERT(NS_IsMainThread(), "Not on main thread"); mClientAuthRemember->ClearRememberedDecisions(); mIOLayerHelpers.clearStoredData(); } +void +SharedSSLState::NoteSocketCreated() +{ + MutexAutoLock lock(mMutex); + mSocketCreated = true; +} + +bool +SharedSSLState::SocketCreated() +{ + MutexAutoLock lock(mMutex); + return mSocketCreated; +} + /*static*/ void SharedSSLState::GlobalInit() { @@ -89,6 +196,18 @@ SharedSSLState::GlobalCleanup() gPublicState = nullptr; } +/*static*/ void +SharedSSLState::NoteCertOverrideServiceInstantiated() +{ + unused << PR_ATOMIC_SET(&sCertOverrideSvcExists, 1); +} + +/*static*/ void +SharedSSLState::NoteCertDBServiceInstantiated() +{ + unused << PR_ATOMIC_SET(&sCertDBExists, 1); +} + void SharedSSLState::Cleanup() { diff --git a/security/manager/ssl/src/SharedSSLState.h b/security/manager/ssl/src/SharedSSLState.h index 7a89fbca243b..d9266d5ba42d 100644 --- a/security/manager/ssl/src/SharedSSLState.h +++ b/security/manager/ssl/src/SharedSSLState.h @@ -11,8 +11,6 @@ #include "nsNSSIOLayer.h" class nsClientAuthRememberService; -class nsIRecentBadCertsService; -class nsICertOverrideService; class nsIObserver; namespace mozilla { @@ -22,6 +20,7 @@ class SharedSSLState { public: NS_INLINE_DECL_THREADSAFE_REFCOUNTING(SharedSSLState) SharedSSLState(); + ~SharedSSLState(); static void GlobalInit(); static void GlobalCleanup(); @@ -34,15 +33,28 @@ public: return mIOLayerHelpers; } + // Main-thread only void ResetStoredData(); void NotePrivateBrowsingStatus(); + // The following methods may be called from any thread + bool SocketCreated(); + void NoteSocketCreated(); + static void NoteCertOverrideServiceInstantiated(); + static void NoteCertDBServiceInstantiated(); + private: void Cleanup(); nsCOMPtr mObserver; RefPtr mClientAuthRemember; nsSSLIOLayerHelpers mIOLayerHelpers; + + // True if any sockets have been created that use this shared data. + // Requires synchronization between the socket and main threads for + // reading/writing. + Mutex mMutex; + bool mSocketCreated; }; SharedSSLState* PublicSSLState(); diff --git a/security/manager/ssl/src/nsCertOverrideService.cpp b/security/manager/ssl/src/nsCertOverrideService.cpp index e2db1ce91dee..e033f74c299b 100644 --- a/security/manager/ssl/src/nsCertOverrideService.cpp +++ b/security/manager/ssl/src/nsCertOverrideService.cpp @@ -19,6 +19,7 @@ #include "nsThreadUtils.h" #include "nsStringBuffer.h" #include "ScopedNSSTypes.h" +#include "SharedSSLState.h" #include "nspr.h" #include "pk11pub.h" @@ -27,6 +28,7 @@ #include "ssl.h" // For SSL_ClearSessionCache using namespace mozilla; +using mozilla::psm::SharedSSLState; static const char kCertOverrideFileName[] = "cert_override.txt"; @@ -124,11 +126,11 @@ nsCertOverrideService::Init() if (observerService) { observerService->AddObserver(this, "profile-before-change", true); observerService->AddObserver(this, "profile-do-change", true); - observerService->AddObserver(this, "last-pb-context-exited", true); // simulate a profile change so we read the current profile's settings file Observe(nullptr, "profile-do-change", nullptr); } + SharedSSLState::NoteCertOverrideServiceInstantiated(); return NS_OK; } @@ -169,10 +171,6 @@ nsCertOverrideService::Observe(nsISupports *, } Read(); - } else if (!nsCRT::strcmp(aTopic, "last-pb-context-exited")) { - ClearValidityOverride( - NS_LITERAL_CSTRING("all:temporary-certificates"), - 0); } return NS_OK; diff --git a/security/manager/ssl/src/nsNSSCertificateDB.cpp b/security/manager/ssl/src/nsNSSCertificateDB.cpp index b3314216a320..d165c0745f96 100644 --- a/security/manager/ssl/src/nsNSSCertificateDB.cpp +++ b/security/manager/ssl/src/nsNSSCertificateDB.cpp @@ -33,6 +33,7 @@ #include "ScopedNSSTypes.h" #include "nsIObserverService.h" #include "nsRecentBadCerts.h" +#include "SharedSSLState.h" #include "nspr.h" #include "certdb.h" @@ -45,6 +46,7 @@ #include "plbase64.h" using namespace mozilla; +using mozilla::psm::SharedSSLState; #ifdef PR_LOGGING extern PRLogModuleInfo* gPIPNSSLog; @@ -56,7 +58,9 @@ static NS_DEFINE_CID(kNSSComponentCID, NS_NSSCOMPONENT_CID); NS_IMPL_THREADSAFE_ISUPPORTS2(nsNSSCertificateDB, nsIX509CertDB, nsIX509CertDB2) nsNSSCertificateDB::nsNSSCertificateDB() +: mBadCertsLock("nsNSSCertificateDB::mBadCertsLock") { + SharedSSLState::NoteCertDBServiceInstantiated(); } nsNSSCertificateDB::~nsNSSCertificateDB() @@ -1650,11 +1654,10 @@ nsNSSCertificateDB::GetCerts(nsIX509CertList **_retval) NS_IMETHODIMP nsNSSCertificateDB::GetRecentBadCerts(bool isPrivate, nsIRecentBadCerts** result) { - MOZ_ASSERT(NS_IsMainThread(), "RecentBadCerts should only be obtained on the main thread"); + MutexAutoLock lock(mBadCertsLock); if (isPrivate) { if (!mPrivateRecentBadCerts) { mPrivateRecentBadCerts = new nsRecentBadCerts; - mPrivateRecentBadCerts->InitPrivateBrowsingObserver(); } NS_ADDREF(*result = mPrivateRecentBadCerts); } else { diff --git a/security/manager/ssl/src/nsNSSCertificateDB.h b/security/manager/ssl/src/nsNSSCertificateDB.h index c821322fbcb4..5ef5015598d1 100644 --- a/security/manager/ssl/src/nsNSSCertificateDB.h +++ b/security/manager/ssl/src/nsNSSCertificateDB.h @@ -8,6 +8,7 @@ #include "nsIX509CertDB.h" #include "nsIX509CertDB2.h" #include "mozilla/RefPtr.h" +#include "mozilla/Mutex.h" #include "certt.h" class nsCString; @@ -51,6 +52,7 @@ private: nsresult handleCACertDownload(nsIArray *x509Certs, nsIInterfaceRequestor *ctx); + mozilla::Mutex mBadCertsLock; mozilla::RefPtr mPublicRecentBadCerts; mozilla::RefPtr mPrivateRecentBadCerts; }; diff --git a/security/manager/ssl/src/nsNSSComponent.cpp b/security/manager/ssl/src/nsNSSComponent.cpp index 31d21ed90ac3..aabe6fab9789 100644 --- a/security/manager/ssl/src/nsNSSComponent.cpp +++ b/security/manager/ssl/src/nsNSSComponent.cpp @@ -2138,7 +2138,6 @@ nsNSSComponent::RandomUpdate(void *entropy, int32_t bufLen) #define PROFILE_CHANGE_TEARDOWN_VETO_TOPIC "profile-change-teardown-veto" #define PROFILE_BEFORE_CHANGE_TOPIC "profile-before-change" #define PROFILE_DO_CHANGE_TOPIC "profile-do-change" -#define LEAVE_PRIVATE_BROWSING_TOPIC "last-pb-context-exited" NS_IMETHODIMP nsNSSComponent::Observe(nsISupports *aSubject, const char *aTopic, @@ -2286,9 +2285,6 @@ nsNSSComponent::Observe(nsISupports *aSubject, const char *aTopic, PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, ("receiving network restore topic\n")); DoProfileChangeNetRestore(); } - else if (nsCRT::strcmp(aTopic, LEAVE_PRIVATE_BROWSING_TOPIC) == 0) { - SSL_ClearSessionCache(); - } return NS_OK; } @@ -2388,7 +2384,6 @@ nsNSSComponent::RegisterObservers() observerService->AddObserver(this, PROFILE_DO_CHANGE_TOPIC, false); observerService->AddObserver(this, PROFILE_CHANGE_NET_TEARDOWN_TOPIC, false); observerService->AddObserver(this, PROFILE_CHANGE_NET_RESTORE_TOPIC, false); - observerService->AddObserver(this, LEAVE_PRIVATE_BROWSING_TOPIC, false); } return NS_OK; } @@ -2414,7 +2409,6 @@ nsNSSComponent::DeregisterObservers() observerService->RemoveObserver(this, PROFILE_DO_CHANGE_TOPIC); observerService->RemoveObserver(this, PROFILE_CHANGE_NET_TEARDOWN_TOPIC); observerService->RemoveObserver(this, PROFILE_CHANGE_NET_RESTORE_TOPIC); - observerService->RemoveObserver(this, LEAVE_PRIVATE_BROWSING_TOPIC); } return NS_OK; } diff --git a/security/manager/ssl/src/nsNSSIOLayer.cpp b/security/manager/ssl/src/nsNSSIOLayer.cpp index 8e84c9647b2b..cecd9d633598 100644 --- a/security/manager/ssl/src/nsNSSIOLayer.cpp +++ b/security/manager/ssl/src/nsNSSIOLayer.cpp @@ -2583,6 +2583,8 @@ nsSSLIOLayerAddToSocket(int32_t family, infoObject->SetHandshakePending(false); } + infoObject->SharedState().NoteSocketCreated(); + return NS_OK; loser: NS_IF_RELEASE(infoObject); diff --git a/security/manager/ssl/src/nsRecentBadCerts.cpp b/security/manager/ssl/src/nsRecentBadCerts.cpp index cae4fd1fb61c..1b121cc767d7 100644 --- a/security/manager/ssl/src/nsRecentBadCerts.cpp +++ b/security/manager/ssl/src/nsRecentBadCerts.cpp @@ -22,9 +22,8 @@ using namespace mozilla; -NS_IMPL_THREADSAFE_ISUPPORTS2(nsRecentBadCerts, - nsIRecentBadCerts, - nsIObserver) +NS_IMPL_THREADSAFE_ISUPPORTS1(nsRecentBadCerts, + nsIRecentBadCerts) nsRecentBadCerts::nsRecentBadCerts() :monitor("nsRecentBadCerts.monitor") @@ -36,24 +35,6 @@ nsRecentBadCerts::~nsRecentBadCerts() { } -void -nsRecentBadCerts::InitPrivateBrowsingObserver() -{ - nsCOMPtr obsSvc = mozilla::services::GetObserverService(); - obsSvc->AddObserver(this, "last-pb-context-exited", false); -} - -NS_IMETHODIMP -nsRecentBadCerts::Observe(nsISupports *aSubject, - const char *aTopic, - const PRUnichar *aData) -{ - if (!nsCRT::strcmp(aTopic, "last-pb-context-exited")) { - ResetStoredCerts(); - } - return NS_OK; -} - NS_IMETHODIMP nsRecentBadCerts::GetRecentBadCert(const nsAString & aHostNameWithPort, nsISSLStatus **aStatus) diff --git a/security/manager/ssl/src/nsRecentBadCerts.h b/security/manager/ssl/src/nsRecentBadCerts.h index d815482d3794..064cb46cd1e8 100644 --- a/security/manager/ssl/src/nsRecentBadCerts.h +++ b/security/manager/ssl/src/nsRecentBadCerts.h @@ -11,7 +11,6 @@ #include "mozilla/ReentrantMonitor.h" #include "nsIRecentBadCertsService.h" -#include "nsIObserver.h" #include "nsTHashtable.h" #include "nsString.h" #include "cert.h" @@ -56,18 +55,14 @@ private: }; class nsRecentBadCerts MOZ_FINAL : public nsIRecentBadCerts - , public nsIObserver { public: NS_DECL_ISUPPORTS NS_DECL_NSIRECENTBADCERTS - NS_DECL_NSIOBSERVER nsRecentBadCerts(); ~nsRecentBadCerts(); - void InitPrivateBrowsingObserver(); - protected: mozilla::ReentrantMonitor monitor;