From 7380482a28d8790367b4f65e12f2d5ab9b3d99df Mon Sep 17 00:00:00 2001 From: David Keeler Date: Mon, 26 Oct 2015 16:02:19 -0700 Subject: [PATCH] bug 1218596 - remove nsPSMInitPanic and other unnecessary things from nsNSSComponent r=Cykesiopka r=jcj --- security/manager/ssl/nsKeygenHandler.cpp | 1 + security/manager/ssl/nsNSSComponent.cpp | 193 ++++++------------ security/manager/ssl/nsNSSComponent.h | 51 +---- .../manager/ssl/nsSiteSecurityService.cpp | 1 + 4 files changed, 73 insertions(+), 173 deletions(-) diff --git a/security/manager/ssl/nsKeygenHandler.cpp b/security/manager/ssl/nsKeygenHandler.cpp index 4df251ea6619..4ec3184fc40b 100644 --- a/security/manager/ssl/nsKeygenHandler.cpp +++ b/security/manager/ssl/nsKeygenHandler.cpp @@ -18,6 +18,7 @@ #include "nsIDOMHTMLSelectElement.h" #include "nsIContent.h" #include "nsKeygenThread.h" +#include "nsNSSHelper.h" #include "nsReadableUtils.h" #include "nsUnicharUtils.h" #include "nsCRT.h" diff --git a/security/manager/ssl/nsNSSComponent.cpp b/security/manager/ssl/nsNSSComponent.cpp index bea8a6f31616..71de64c6b8b7 100644 --- a/security/manager/ssl/nsNSSComponent.cpp +++ b/security/manager/ssl/nsNSSComponent.cpp @@ -8,55 +8,49 @@ #include "ExtendedValidation.h" #include "NSSCertDBTrustDomain.h" -#include "mozilla/Telemetry.h" -#include "nsAppDirectoryServiceDefs.h" -#include "nsCertVerificationThread.h" -#include "nsAppDirectoryServiceDefs.h" -#include "nsComponentManagerUtils.h" -#include "nsDirectoryServiceDefs.h" -#include "nsICertOverrideService.h" -#include "NSSCertDBTrustDomain.h" -#include "nsThreadUtils.h" +#include "SharedSSLState.h" #include "mozilla/Preferences.h" -#include "nsThreadUtils.h" #include "mozilla/PublicSSL.h" #include "mozilla/Services.h" #include "mozilla/StaticPtr.h" +#include "mozilla/Telemetry.h" +#include "nsAppDirectoryServiceDefs.h" +#include "nsCRT.h" +#include "nsCertVerificationThread.h" +#include "nsClientAuthRemember.h" +#include "nsComponentManagerUtils.h" +#include "nsDirectoryServiceDefs.h" +#include "nsIBufEntropyCollector.h" +#include "nsICertOverrideService.h" +#include "nsIFile.h" +#include "nsIObserverService.h" +#include "nsIPrompt.h" +#include "nsIProperties.h" +#include "nsISiteSecurityService.h" +#include "nsITokenPasswordDialogs.h" +#include "nsIWindowWatcher.h" +#include "nsNSSHelper.h" +#include "nsNSSShutDown.h" +#include "nsServiceManagerUtils.h" +#include "nsThreadUtils.h" +#include "nsXULAppAPI.h" +#include "nss.h" +#include "p12plcy.h" +#include "pkix/pkixnss.h" +#include "secerr.h" +#include "secmod.h" +#include "ssl.h" +#include "sslerr.h" +#include "sslproto.h" #ifndef MOZ_NO_SMART_CARDS #include "nsSmartCardMonitor.h" #endif -#include "nsCRT.h" -#include "nsNTLMAuthModule.h" -#include "nsIFile.h" -#include "nsIProperties.h" -#include "nsIWindowWatcher.h" -#include "nsIPrompt.h" -#include "nsIBufEntropyCollector.h" -#include "nsITokenPasswordDialogs.h" -#include "nsISiteSecurityService.h" -#include "nsServiceManagerUtils.h" -#include "nsNSSShutDown.h" -#include "SharedSSLState.h" -#include "NSSErrorsService.h" - -#include "nss.h" -#include "pkix/pkixnss.h" -#include "ssl.h" -#include "sslproto.h" -#include "secmod.h" -#include "secerr.h" -#include "sslerr.h" - -#include "nsXULAppAPI.h" - #ifdef XP_WIN #include "nsILocalFileWin.h" #endif -#include "p12plcy.h" - using namespace mozilla; using namespace mozilla::psm; @@ -64,8 +58,6 @@ PRLogModuleInfo* gPIPNSSLog = nullptr; int nsNSSComponent::mInstanceCount = 0; -bool nsPSMInitPanic::isPanic = false; - // This function can be called from chrome or content processes // to ensure that NSS is initialized. bool EnsureNSSInitializedChromeOrContent() @@ -104,9 +96,6 @@ bool EnsureNSSInitializedChromeOrContent() // creating any other components. bool EnsureNSSInitialized(EnsureNSSOperator op) { - if (nsPSMInitPanic::GetPanic()) - return false; - if (GeckoProcessType_Default != XRE_GetProcessType()) { if (op == nssEnsureOnChromeOnly) @@ -234,7 +223,6 @@ nsNSSComponent::nsNSSComponent() if (!gPIPNSSLog) gPIPNSSLog = PR_NewLogModule("pipnss"); MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsNSSComponent::ctor\n")); - mObserversRegistered = false; NS_ASSERTION( (0 == mInstanceCount), "nsNSSComponent is a singleton, but instantiated multiple times!"); ++mInstanceCount; @@ -331,26 +319,6 @@ nsNSSComponent::GetPIPNSSBundleString(const char* name, nsAString& outString) return rv; } -NS_IMETHODIMP -nsNSSComponent::NSSBundleFormatStringFromName(const char* name, - const char16_t** params, - uint32_t numParams, - nsAString& outString) -{ - nsresult rv = NS_ERROR_FAILURE; - - if (mNSSErrorsBundle && name) { - nsXPIDLString result; - rv = mNSSErrorsBundle->FormatStringFromName(NS_ConvertASCIItoUTF16(name).get(), - params, numParams, - getter_Copies(result)); - if (NS_SUCCEEDED(rv)) { - outString = result; - } - } - return rv; -} - NS_IMETHODIMP nsNSSComponent::GetNSSBundleString(const char* name, nsAString& outString) { @@ -1016,7 +984,6 @@ nsNSSComponent::InitializeNSS() nsAutoCString profileStr; nsresult rv = GetNSSProfilePath(profileStr); if (NS_FAILED(rv)) { - nsPSMInitPanic::SetPanic(); return NS_ERROR_NOT_AVAILABLE; } @@ -1043,7 +1010,6 @@ nsNSSComponent::InitializeNSS() } if (init_rv != SECSuccess) { MOZ_LOG(gPIPNSSLog, LogLevel::Error, ("could not initialize NSS - panicking\n")); - nsPSMInitPanic::SetPanic(); return NS_ERROR_NOT_AVAILABLE; } @@ -1061,7 +1027,6 @@ nsNSSComponent::InitializeNSS() rv = setEnabledTLSVersions(); if (NS_FAILED(rv)) { - nsPSMInitPanic::SetPanic(); return NS_ERROR_UNEXPECTED; } @@ -1166,7 +1131,7 @@ nsNSSComponent::ShutdownNSS() } } -NS_IMETHODIMP +nsresult nsNSSComponent::Init() { // No mutex protection. @@ -1201,54 +1166,42 @@ nsNSSComponent::Init() getter_Copies(result)); } - // Do that before NSS init, to make sure we won't get unloaded. - RegisterObservers(); rv = InitializeNSS(); if (NS_FAILED(rv)) { - MOZ_LOG(gPIPNSSLog, LogLevel::Error, ("Unable to Initialize NSS.\n")); - - DeregisterObservers(); - mPIPNSSBundle = nullptr; + MOZ_LOG(gPIPNSSLog, LogLevel::Error, + ("nsNSSComponent::InitializeNSS() failed\n")); return rv; } RememberCertErrorsTable::Init(); createBackgroundThreads(); - if (!mCertVerificationThread) - { - MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("NSS init, could not create threads\n")); - - DeregisterObservers(); - mPIPNSSBundle = nullptr; + if (!mCertVerificationThread) { + MOZ_LOG(gPIPNSSLog, LogLevel::Debug, + ("nsNSSComponent::createBackgroundThreads() failed\n")); return NS_ERROR_OUT_OF_MEMORY; } - nsCOMPtr ec - = do_GetService(NS_ENTROPYCOLLECTOR_CONTRACTID); - - nsCOMPtr bec; - - if (ec) { - bec = do_QueryInterface(ec); + nsCOMPtr ec( + do_GetService(NS_ENTROPYCOLLECTOR_CONTRACTID)); + if (!ec) { + return NS_ERROR_FAILURE; } - - NS_ASSERTION(bec, "No buffering entropy collector. " - "This means no entropy will be collected."); - if (bec) { - bec->ForwardTo(this); + nsCOMPtr bec(do_QueryInterface(ec)); + if (!bec) { + return NS_ERROR_FAILURE; } + bec->ForwardTo(this); - return rv; + return RegisterObservers(); } // nsISupports Implementation for the class NS_IMPL_ISUPPORTS(nsNSSComponent, nsIEntropyCollector, nsINSSComponent, - nsIObserver, - nsISupportsWeakReference) + nsIObserver) NS_IMETHODIMP nsNSSComponent::RandomUpdate(void* entropy, int32_t bufLen) @@ -1454,48 +1407,24 @@ nsNSSComponent::RegisterObservers() { // Happens once during init only, no mutex protection. - nsCOMPtr observerService(do_GetService("@mozilla.org/observer-service;1")); - NS_ASSERTION(observerService, "could not get observer service"); - if (observerService) { - mObserversRegistered = true; - MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsNSSComponent: adding observers\n")); - - // We are a service. - // Once we are loaded, don't allow being removed from memory. - // This makes sense, as initializing NSS is expensive. - - // By using false for parameter ownsWeak in AddObserver, - // we make sure that we won't get unloaded until the application shuts down. - - observerService->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false); - - observerService->AddObserver(this, PROFILE_BEFORE_CHANGE_TOPIC, false); - 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); + nsCOMPtr observerService( + do_GetService("@mozilla.org/observer-service;1")); + if (!observerService) { + MOZ_LOG(gPIPNSSLog, LogLevel::Debug, + ("nsNSSComponent: couldn't get observer service\n")); + return NS_ERROR_FAILURE; } - return NS_OK; -} -nsresult -nsNSSComponent::DeregisterObservers() -{ - if (!mObserversRegistered) - return NS_OK; + MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsNSSComponent: adding observers\n")); + // Using false for the ownsweak parameter means the observer service will + // keep a strong reference to this component. As a result, this will live at + // least as long as the observer service. + observerService->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false); + observerService->AddObserver(this, PROFILE_BEFORE_CHANGE_TOPIC, false); + 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); - nsCOMPtr observerService(do_GetService("@mozilla.org/observer-service;1")); - NS_ASSERTION(observerService, "could not get observer service"); - if (observerService) { - mObserversRegistered = false; - MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsNSSComponent: removing observers\n")); - - observerService->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID); - - observerService->RemoveObserver(this, PROFILE_BEFORE_CHANGE_TOPIC); - observerService->RemoveObserver(this, PROFILE_DO_CHANGE_TOPIC); - observerService->RemoveObserver(this, PROFILE_CHANGE_NET_TEARDOWN_TOPIC); - observerService->RemoveObserver(this, PROFILE_CHANGE_NET_RESTORE_TOPIC); - } return NS_OK; } diff --git a/security/manager/ssl/nsNSSComponent.h b/security/manager/ssl/nsNSSComponent.h index fa1d877bdb48..277d13922c42 100644 --- a/security/manager/ssl/nsNSSComponent.h +++ b/security/manager/ssl/nsNSSComponent.h @@ -13,12 +13,8 @@ #include "nsIEntropyCollector.h" #include "nsIStringBundle.h" #include "nsIObserver.h" -#include "nsIObserverService.h" -#include "nsINSSErrorsService.h" #include "nsNSSCallbacks.h" #include "SharedCertVerifier.h" -#include "nsNSSHelper.h" -#include "nsClientAuthRemember.h" #include "prerror.h" #include "sslt.h" @@ -40,13 +36,9 @@ MOZ_WARN_UNUSED_RESULT #define PSM_COMPONENT_CONTRACTID "@mozilla.org/psm;1" -//Define an interface that we can use to look up from the -//callbacks passed to NSS. - -#define NS_INSSCOMPONENT_IID_STR "e60602a8-97a3-4fe7-b5b7-56bc6ce87ab4" #define NS_INSSCOMPONENT_IID \ - { 0xe60602a8, 0x97a3, 0x4fe7, \ - { 0xb5, 0xb7, 0x56, 0xbc, 0x6c, 0xe8, 0x7a, 0xb4 } } + { 0xa0a8f52b, 0xea18, 0x4abc, \ + { 0xa3, 0xca, 0xec, 0xcf, 0x70, 0x4f, 0xfe, 0x63 } } enum EnsureNSSOperator { @@ -63,10 +55,9 @@ extern bool EnsureNSSInitializedChromeOrContent(); extern bool EnsureNSSInitialized(EnsureNSSOperator op); -class nsNSSComponent; - -class NS_NO_VTABLE nsINSSComponent : public nsISupports { - public: +class NS_NO_VTABLE nsINSSComponent : public nsISupports +{ +public: NS_DECLARE_STATIC_IID_ACCESSOR(NS_INSSCOMPONENT_IID) NS_IMETHOD ShowAlertFromStringBundle(const char* messageID) = 0; @@ -80,10 +71,6 @@ class NS_NO_VTABLE nsINSSComponent : public nsISupports { NS_IMETHOD GetNSSBundleString(const char* name, nsAString& outString) = 0; - NS_IMETHOD NSSBundleFormatStringFromName(const char* name, - const char16_t** params, - uint32_t numParams, - nsAString& outString) = 0; NS_IMETHOD LogoutAuthenticatedPK11() = 0; @@ -105,13 +92,10 @@ class nsNSSShutDownList; class nsCertVerificationThread; // Implementation of the PSM component interface. -class nsNSSComponent final : public nsIEntropyCollector, - public nsINSSComponent, - public nsIObserver, - public nsSupportsWeakReference +class nsNSSComponent final : public nsIEntropyCollector + , public nsINSSComponent + , public nsIObserver { - typedef mozilla::Mutex Mutex; - public: NS_DEFINE_STATIC_CID_ACCESSOR( NS_NSSCOMPONENT_CID ) @@ -121,7 +105,7 @@ public: NS_DECL_NSIENTROPYCOLLECTOR NS_DECL_NSIOBSERVER - NS_METHOD Init(); + nsresult Init(); static nsresult GetNewPrompter(nsIPrompt** result); static nsresult ShowAlertWithConstructedString(const nsString& message); @@ -134,10 +118,6 @@ public: uint32_t numParams, nsAString& outString) override; NS_IMETHOD GetNSSBundleString(const char* name, nsAString& outString) override; - NS_IMETHOD NSSBundleFormatStringFromName(const char* name, - const char16_t** params, - uint32_t numParams, - nsAString& outString) override; NS_IMETHOD LogoutAuthenticatedPK11() override; #ifndef MOZ_NO_SMART_CARDS @@ -179,7 +159,6 @@ private: nsresult InitializePIPNSSBundle(); nsresult ConfigureInternalPKCS11Token(); nsresult RegisterObservers(); - nsresult DeregisterObservers(); // Methods that we use to handle the profile change notifications (and to // synthesize a full profile change when we're just doing a profile startup): @@ -187,12 +166,11 @@ private: void DoProfileBeforeChange(nsISupports* aSubject); void DoProfileChangeNetRestore(); - Mutex mutex; + mozilla::Mutex mutex; nsCOMPtr mPIPNSSBundle; nsCOMPtr mNSSErrorsBundle; bool mNSSInitialized; - bool mObserversRegistered; static int mInstanceCount; nsNSSShutDownList* mShutdownObjectList; #ifndef MOZ_NO_SMART_CARDS @@ -220,13 +198,4 @@ public: nsString& returnedMessage); }; -class nsPSMInitPanic -{ -private: - static bool isPanic; -public: - static void SetPanic() {isPanic = true;} - static bool GetPanic() {return isPanic;} -}; - #endif // _nsNSSComponent_h_ diff --git a/security/manager/ssl/nsSiteSecurityService.cpp b/security/manager/ssl/nsSiteSecurityService.cpp index 407cb2677900..46f0ce7140bb 100644 --- a/security/manager/ssl/nsSiteSecurityService.cpp +++ b/security/manager/ssl/nsSiteSecurityService.cpp @@ -13,6 +13,7 @@ #include "nsISSLStatus.h" #include "nsISocketProvider.h" #include "nsIURI.h" +#include "nsIX509Cert.h" #include "nsNetUtil.h" #include "nsNSSComponent.h" #include "nsSecurityHeaderParser.h"